]> git.ipfire.org Git - thirdparty/git.git/commitdiff
Merge branch 'ps/reftable-iteration-perf-part2'
authorJunio C Hamano <gitster@pobox.com>
Thu, 14 Mar 2024 21:05:23 +0000 (14:05 -0700)
committerJunio C Hamano <gitster@pobox.com>
Thu, 14 Mar 2024 21:05:23 +0000 (14:05 -0700)
The code to iterate over refs with the reftable backend has seen
some optimization.

* ps/reftable-iteration-perf-part2:
  refs/reftable: precompute prefix length
  reftable: allow inlining of a few functions
  reftable/record: decode keys in place
  reftable/record: reuse refname when copying
  reftable/record: reuse refname when decoding
  reftable/merged: avoid duplicate pqueue emptiness check
  reftable/merged: circumvent pqueue with single subiter
  reftable/merged: handle subiter cleanup on close only
  reftable/merged: remove unnecessary null check for subiters
  reftable/merged: make subiters own their records
  reftable/merged: advance subiter on subsequent iteration
  reftable/merged: make `merged_iter` structure private
  reftable/pq: use `size_t` to track iterator index

14 files changed:
refs/reftable-backend.c
reftable/block.c
reftable/block.h
reftable/iter.c
reftable/iter.h
reftable/merged.c
reftable/merged.h
reftable/pq.c
reftable/pq.h
reftable/pq_test.c
reftable/record.c
reftable/record.h
reftable/record_test.c
reftable/reftable-record.h

index 2c88bbd448b18785200a83b7a9e4e794cc364d60..74dab18eda50f8158f3e8c133a8dde70299ce3cb 100644 (file)
@@ -346,6 +346,7 @@ struct reftable_ref_iterator {
        struct object_id oid;
 
        const char *prefix;
+       size_t prefix_len;
        unsigned int flags;
        int err;
 };
@@ -374,8 +375,8 @@ static int reftable_ref_iterator_advance(struct ref_iterator *ref_iterator)
                        continue;
                }
 
-               if (iter->prefix &&
-                   strncmp(iter->prefix, iter->ref.refname, strlen(iter->prefix))) {
+               if (iter->prefix_len &&
+                   strncmp(iter->prefix, iter->ref.refname, iter->prefix_len)) {
                        iter->err = 1;
                        break;
                }
@@ -484,6 +485,7 @@ static struct reftable_ref_iterator *ref_iterator_for_stack(struct reftable_ref_
        iter = xcalloc(1, sizeof(*iter));
        base_ref_iterator_init(&iter->base, &reftable_ref_iterator_vtable);
        iter->prefix = prefix;
+       iter->prefix_len = prefix ? strlen(prefix) : 0;
        iter->base.oid = &iter->oid;
        iter->flags = flags;
        iter->refs = refs;
index 72eb73b3806d623010c0e224f4f8ce77d555ce43..ad9074dba6121e9e40265f73494b233d99fa74d4 100644 (file)
@@ -291,9 +291,8 @@ static int restart_key_less(size_t idx, void *args)
        /* the restart key is verbatim in the block, so this could avoid the
           alloc for decoding the key */
        struct strbuf rkey = STRBUF_INIT;
-       struct strbuf last_key = STRBUF_INIT;
        uint8_t unused_extra;
-       int n = reftable_decode_key(&rkey, &unused_extra, last_key, in);
+       int n = reftable_decode_key(&rkey, &unused_extra, in);
        int result;
        if (n < 0) {
                a->error = 1;
@@ -326,35 +325,34 @@ int block_iter_next(struct block_iter *it, struct reftable_record *rec)
        if (it->next_off >= it->br->block_len)
                return 1;
 
-       n = reftable_decode_key(&it->key, &extra, it->last_key, in);
+       n = reftable_decode_key(&it->last_key, &extra, in);
        if (n < 0)
                return -1;
-
-       if (!it->key.len)
+       if (!it->last_key.len)
                return REFTABLE_FORMAT_ERROR;
 
        string_view_consume(&in, n);
-       n = reftable_record_decode(rec, it->key, extra, in, it->br->hash_size);
+       n = reftable_record_decode(rec, it->last_key, extra, in, it->br->hash_size);
        if (n < 0)
                return -1;
        string_view_consume(&in, n);
 
-       strbuf_swap(&it->last_key, &it->key);
        it->next_off += start.len - in.len;
        return 0;
 }
 
 int block_reader_first_key(struct block_reader *br, struct strbuf *key)
 {
-       struct strbuf empty = STRBUF_INIT;
-       int off = br->header_off + 4;
+       int off = br->header_off + 4, n;
        struct string_view in = {
                .buf = br->block.data + off,
                .len = br->block_len - off,
        };
-
        uint8_t extra = 0;
-       int n = reftable_decode_key(key, &extra, empty, in);
+
+       strbuf_reset(key);
+
+       n = reftable_decode_key(key, &extra, in);
        if (n < 0)
                return n;
        if (!key->len)
@@ -371,7 +369,6 @@ int block_iter_seek(struct block_iter *it, struct strbuf *want)
 void block_iter_close(struct block_iter *it)
 {
        strbuf_release(&it->last_key);
-       strbuf_release(&it->key);
 }
 
 int block_reader_seek(struct block_reader *br, struct block_iter *it,
@@ -408,8 +405,8 @@ int block_reader_seek(struct block_reader *br, struct block_iter *it,
                if (err < 0)
                        goto done;
 
-               reftable_record_key(&rec, &it->key);
-               if (err > 0 || strbuf_cmp(&it->key, want) >= 0) {
+               reftable_record_key(&rec, &it->last_key);
+               if (err > 0 || strbuf_cmp(&it->last_key, want) >= 0) {
                        err = 0;
                        goto done;
                }
index 17481e6331979cc31972ee3dad576e9c594a1769..51699af233d823fafe681f3d7cbb20cf61b88d8a 100644 (file)
@@ -84,12 +84,10 @@ struct block_iter {
 
        /* key for last entry we read. */
        struct strbuf last_key;
-       struct strbuf key;
 };
 
 #define BLOCK_ITER_INIT { \
        .last_key = STRBUF_INIT, \
-       .key = STRBUF_INIT, \
 }
 
 /* initializes a block reader. */
index 8b5ebf6183a97f3ea16a0773880ccd3b835ef68e..7aa30c4a51e54ec5442b9d8a3bf90b8119c5a4f0 100644 (file)
@@ -16,11 +16,6 @@ https://developers.google.com/open-source/licenses/bsd
 #include "reader.h"
 #include "reftable-error.h"
 
-int iterator_is_null(struct reftable_iterator *it)
-{
-       return !it->ops;
-}
-
 static void filtering_ref_iterator_close(void *iter_arg)
 {
        struct filtering_ref_iterator *fri = iter_arg;
index 47d67d84df679c522ce50d1d7aade2d1be683b5a..537431baba075ad72614b1c68eff1fd28fa51288 100644 (file)
@@ -16,10 +16,6 @@ https://developers.google.com/open-source/licenses/bsd
 #include "reftable-iterator.h"
 #include "reftable-generic.h"
 
-/* Returns true for a zeroed out iterator, such as the one returned from
- * iterator_destroy. */
-int iterator_is_null(struct reftable_iterator *it);
-
 /* iterator that produces only ref records that point to `oid` */
 struct filtering_ref_iterator {
        int double_check;
index 1aa6cd31b74954da9b1b5140cd350459c37c1378..f85a24c6786c6fb8e102ffc9ceb35f23706c8c9b 100644 (file)
@@ -17,23 +17,37 @@ https://developers.google.com/open-source/licenses/bsd
 #include "reftable-error.h"
 #include "system.h"
 
+struct merged_subiter {
+       struct reftable_iterator iter;
+       struct reftable_record rec;
+};
+
+struct merged_iter {
+       struct merged_subiter *subiters;
+       struct merged_iter_pqueue pq;
+       uint32_t hash_id;
+       size_t stack_len;
+       uint8_t typ;
+       int suppress_deletions;
+       ssize_t advance_index;
+};
+
 static int merged_iter_init(struct merged_iter *mi)
 {
        for (size_t i = 0; i < mi->stack_len; i++) {
                struct pq_entry e = {
                        .index = i,
+                       .rec = &mi->subiters[i].rec,
                };
                int err;
 
-               reftable_record_init(&e.rec, mi->typ);
-               err = iterator_next(&mi->stack[i], &e.rec);
+               reftable_record_init(&mi->subiters[i].rec, mi->typ);
+               err = iterator_next(&mi->subiters[i].iter,
+                                   &mi->subiters[i].rec);
                if (err < 0)
                        return err;
-               if (err > 0) {
-                       reftable_iterator_destroy(&mi->stack[i]);
-                       reftable_record_release(&e.rec);
+               if (err > 0)
                        continue;
-               }
 
                merged_iter_pqueue_add(&mi->pq, &e);
        }
@@ -46,54 +60,66 @@ static void merged_iter_close(void *p)
        struct merged_iter *mi = p;
 
        merged_iter_pqueue_release(&mi->pq);
-       for (size_t i = 0; i < mi->stack_len; i++)
-               reftable_iterator_destroy(&mi->stack[i]);
-       reftable_free(mi->stack);
+       for (size_t i = 0; i < mi->stack_len; i++) {
+               reftable_iterator_destroy(&mi->subiters[i].iter);
+               reftable_record_release(&mi->subiters[i].rec);
+       }
+       reftable_free(mi->subiters);
 }
 
-static int merged_iter_advance_nonnull_subiter(struct merged_iter *mi,
-                                              size_t idx)
+static int merged_iter_advance_subiter(struct merged_iter *mi, size_t idx)
 {
        struct pq_entry e = {
                .index = idx,
+               .rec = &mi->subiters[idx].rec,
        };
        int err;
 
-       reftable_record_init(&e.rec, mi->typ);
-       err = iterator_next(&mi->stack[idx], &e.rec);
-       if (err < 0)
+       err = iterator_next(&mi->subiters[idx].iter, &mi->subiters[idx].rec);
+       if (err)
                return err;
 
-       if (err > 0) {
-               reftable_iterator_destroy(&mi->stack[idx]);
-               reftable_record_release(&e.rec);
-               return 0;
-       }
-
        merged_iter_pqueue_add(&mi->pq, &e);
        return 0;
 }
 
-static int merged_iter_advance_subiter(struct merged_iter *mi, size_t idx)
-{
-       if (iterator_is_null(&mi->stack[idx]))
-               return 0;
-       return merged_iter_advance_nonnull_subiter(mi, idx);
-}
-
 static int merged_iter_next_entry(struct merged_iter *mi,
                                  struct reftable_record *rec)
 {
        struct pq_entry entry = { 0 };
-       int err = 0;
+       int err = 0, empty;
+
+       empty = merged_iter_pqueue_is_empty(mi->pq);
+
+       if (mi->advance_index >= 0) {
+               /*
+                * When there are no pqueue entries then we only have a single
+                * subiter left. There is no need to use the pqueue in that
+                * case anymore as we know that the subiter will return entries
+                * in the correct order already.
+                *
+                * While this may sound like a very specific edge case, it may
+                * happen more frequently than you think. Most repositories
+                * will end up having a single large base table that contains
+                * most of the refs. It's thus likely that we exhaust all
+                * subiters but the one from that base ref.
+                */
+               if (empty)
+                       return iterator_next(&mi->subiters[mi->advance_index].iter,
+                                            rec);
+
+               err = merged_iter_advance_subiter(mi, mi->advance_index);
+               if (err < 0)
+                       return err;
+               if (!err)
+                       empty = 0;
+               mi->advance_index = -1;
+       }
 
-       if (merged_iter_pqueue_is_empty(mi->pq))
+       if (empty)
                return 1;
 
        entry = merged_iter_pqueue_remove(&mi->pq);
-       err = merged_iter_advance_subiter(mi, entry.index);
-       if (err < 0)
-               return err;
 
        /*
          One can also use reftable as datacenter-local storage, where the ref
@@ -107,56 +133,34 @@ static int merged_iter_next_entry(struct merged_iter *mi,
                struct pq_entry top = merged_iter_pqueue_top(mi->pq);
                int cmp;
 
-               /*
-                * When the next entry comes from the same queue as the current
-                * entry then it must by definition be larger. This avoids a
-                * comparison in the most common case.
-                */
-               if (top.index == entry.index)
-                       break;
-
-               cmp = reftable_record_cmp(&top.rec, &entry.rec);
+               cmp = reftable_record_cmp(top.rec, entry.rec);
                if (cmp > 0)
                        break;
 
                merged_iter_pqueue_remove(&mi->pq);
                err = merged_iter_advance_subiter(mi, top.index);
                if (err < 0)
-                       goto done;
-               reftable_record_release(&top.rec);
+                       return err;
        }
 
-       reftable_record_release(rec);
-       *rec = entry.rec;
-
-done:
-       if (err)
-               reftable_record_release(&entry.rec);
-       return err;
+       mi->advance_index = entry.index;
+       SWAP(*rec, *entry.rec);
+       return 0;
 }
 
-static int merged_iter_next(struct merged_iter *mi, struct reftable_record *rec)
+static int merged_iter_next_void(void *p, struct reftable_record *rec)
 {
+       struct merged_iter *mi = p;
        while (1) {
                int err = merged_iter_next_entry(mi, rec);
-               if (err == 0 && mi->suppress_deletions &&
-                   reftable_record_is_deletion(rec)) {
+               if (err)
+                       return err;
+               if (mi->suppress_deletions && reftable_record_is_deletion(rec))
                        continue;
-               }
-
-               return err;
+               return 0;
        }
 }
 
-static int merged_iter_next_void(void *p, struct reftable_record *rec)
-{
-       struct merged_iter *mi = p;
-       if (merged_iter_pqueue_is_empty(mi->pq))
-               return 1;
-
-       return merged_iter_next(mi, rec);
-}
-
 static struct reftable_iterator_vtable merged_iter_vtable = {
        .next = &merged_iter_next_void,
        .close = &merged_iter_close,
@@ -246,14 +250,15 @@ static int merged_table_seek_record(struct reftable_merged_table *mt,
                .typ = reftable_record_type(rec),
                .hash_id = mt->hash_id,
                .suppress_deletions = mt->suppress_deletions,
+               .advance_index = -1,
        };
        struct merged_iter *p;
        int err;
 
-       REFTABLE_CALLOC_ARRAY(merged.stack, mt->stack_len);
+       REFTABLE_CALLOC_ARRAY(merged.subiters, mt->stack_len);
        for (size_t i = 0; i < mt->stack_len; i++) {
                err = reftable_table_seek_record(&mt->stack[i],
-                                                &merged.stack[merged.stack_len], rec);
+                                                &merged.subiters[merged.stack_len].iter, rec);
                if (err < 0)
                        goto out;
                if (!err)
index 7d9f95d27ed0a44c4208e743b6b83ba4cdc5536c..a2571dbc99d68c9954e9d07372bae984cf5ccccc 100644 (file)
@@ -9,7 +9,7 @@ https://developers.google.com/open-source/licenses/bsd
 #ifndef MERGED_H
 #define MERGED_H
 
-#include "pq.h"
+#include "system.h"
 
 struct reftable_merged_table {
        struct reftable_table *stack;
@@ -24,15 +24,6 @@ struct reftable_merged_table {
        uint64_t max;
 };
 
-struct merged_iter {
-       struct reftable_iterator *stack;
-       uint32_t hash_id;
-       size_t stack_len;
-       uint8_t typ;
-       int suppress_deletions;
-       struct merged_iter_pqueue pq;
-};
-
 void merged_table_release(struct reftable_merged_table *mt);
 
 #endif
index e0ccce2b9779a8bfd1acac6d42bc7d0e46e3fe0e..7fb45d8c60dbca6106c51732005ce15c59d2c7e0 100644 (file)
@@ -14,22 +14,12 @@ https://developers.google.com/open-source/licenses/bsd
 
 int pq_less(struct pq_entry *a, struct pq_entry *b)
 {
-       int cmp = reftable_record_cmp(&a->rec, &b->rec);
+       int cmp = reftable_record_cmp(a->rec, b->rec);
        if (cmp == 0)
                return a->index > b->index;
        return cmp < 0;
 }
 
-struct pq_entry merged_iter_pqueue_top(struct merged_iter_pqueue pq)
-{
-       return pq.heap[0];
-}
-
-int merged_iter_pqueue_is_empty(struct merged_iter_pqueue pq)
-{
-       return pq.len == 0;
-}
-
 struct pq_entry merged_iter_pqueue_remove(struct merged_iter_pqueue *pq)
 {
        int i = 0;
@@ -82,10 +72,6 @@ void merged_iter_pqueue_add(struct merged_iter_pqueue *pq, const struct pq_entry
 
 void merged_iter_pqueue_release(struct merged_iter_pqueue *pq)
 {
-       int i = 0;
-       for (i = 0; i < pq->len; i++) {
-               reftable_record_release(&pq->heap[i].rec);
-       }
        FREE_AND_NULL(pq->heap);
-       pq->len = pq->cap = 0;
+       memset(pq, 0, sizeof(*pq));
 }
index e85bac9b52e0039bd378cb1073215af4d48196c7..f796c2317948be2c82aaa15c29b5e9aba1730168 100644 (file)
@@ -12,8 +12,8 @@ https://developers.google.com/open-source/licenses/bsd
 #include "record.h"
 
 struct pq_entry {
-       int index;
-       struct reftable_record rec;
+       size_t index;
+       struct reftable_record *rec;
 };
 
 struct merged_iter_pqueue {
@@ -22,12 +22,20 @@ struct merged_iter_pqueue {
        size_t cap;
 };
 
-struct pq_entry merged_iter_pqueue_top(struct merged_iter_pqueue pq);
-int merged_iter_pqueue_is_empty(struct merged_iter_pqueue pq);
 void merged_iter_pqueue_check(struct merged_iter_pqueue pq);
 struct pq_entry merged_iter_pqueue_remove(struct merged_iter_pqueue *pq);
 void merged_iter_pqueue_add(struct merged_iter_pqueue *pq, const struct pq_entry *e);
 void merged_iter_pqueue_release(struct merged_iter_pqueue *pq);
 int pq_less(struct pq_entry *a, struct pq_entry *b);
 
+static inline struct pq_entry merged_iter_pqueue_top(struct merged_iter_pqueue pq)
+{
+       return pq.heap[0];
+}
+
+static inline int merged_iter_pqueue_is_empty(struct merged_iter_pqueue pq)
+{
+       return pq.len == 0;
+}
+
 #endif
index c202eff84801820b28c56ddb7de28a2d041f3c08..b7d3c80cc7260298749800696e4a8bcc5be3284c 100644 (file)
@@ -27,48 +27,43 @@ void merged_iter_pqueue_check(struct merged_iter_pqueue pq)
 
 static void test_pq(void)
 {
-       char *names[54] = { NULL };
-       int N = ARRAY_SIZE(names) - 1;
-
        struct merged_iter_pqueue pq = { NULL };
+       struct reftable_record recs[54];
+       int N = ARRAY_SIZE(recs) - 1, i;
        char *last = NULL;
 
-       int i = 0;
        for (i = 0; i < N; i++) {
-               char name[100];
-               snprintf(name, sizeof(name), "%02d", i);
-               names[i] = xstrdup(name);
+               struct strbuf refname = STRBUF_INIT;
+               strbuf_addf(&refname, "%02d", i);
+
+               reftable_record_init(&recs[i], BLOCK_TYPE_REF);
+               recs[i].u.ref.refname = strbuf_detach(&refname, NULL);
        }
 
        i = 1;
        do {
-               struct pq_entry e = { .rec = { .type = BLOCK_TYPE_REF,
-                                              .u.ref = {
-                                                      .refname = names[i],
-                                              } } };
+               struct pq_entry e = {
+                       .rec = &recs[i],
+               };
+
                merged_iter_pqueue_add(&pq, &e);
                merged_iter_pqueue_check(pq);
+
                i = (i * 7) % N;
        } while (i != 1);
 
        while (!merged_iter_pqueue_is_empty(pq)) {
                struct pq_entry e = merged_iter_pqueue_remove(&pq);
-               struct reftable_record *rec = &e.rec;
                merged_iter_pqueue_check(pq);
 
-               EXPECT(reftable_record_type(rec) == BLOCK_TYPE_REF);
-               if (last) {
-                       EXPECT(strcmp(last, rec->u.ref.refname) < 0);
-               }
-               /* this is names[i], so don't dealloc. */
-               last = rec->u.ref.refname;
-               rec->u.ref.refname = NULL;
-               reftable_record_release(rec);
-       }
-       for (i = 0; i < N; i++) {
-               reftable_free(names[i]);
+               EXPECT(reftable_record_type(e.rec) == BLOCK_TYPE_REF);
+               if (last)
+                       EXPECT(strcmp(last, e.rec->u.ref.refname) < 0);
+               last = e.rec->u.ref.refname;
        }
 
+       for (i = 0; i < N; i++)
+               reftable_record_release(&recs[i]);
        merged_iter_pqueue_release(&pq);
 }
 
index d6bb42e887423d95f2fcc1b2b146505ab4bef62a..367de046006b25b497eb82fb35281e98733f609e 100644 (file)
@@ -159,20 +159,19 @@ int reftable_encode_key(int *restart, struct string_view dest,
        return start.len - dest.len;
 }
 
-int reftable_decode_key(struct strbuf *key, uint8_t *extra,
-                       struct strbuf last_key, struct string_view in)
+int reftable_decode_key(struct strbuf *last_key, uint8_t *extra,
+                       struct string_view in)
 {
        int start_len = in.len;
        uint64_t prefix_len = 0;
        uint64_t suffix_len = 0;
-       int n = get_var_int(&prefix_len, &in);
+       int n;
+
+       n = get_var_int(&prefix_len, &in);
        if (n < 0)
                return -1;
        string_view_consume(&in, n);
 
-       if (prefix_len > last_key.len)
-               return -1;
-
        n = get_var_int(&suffix_len, &in);
        if (n <= 0)
                return -1;
@@ -181,12 +180,12 @@ int reftable_decode_key(struct strbuf *key, uint8_t *extra,
        *extra = (uint8_t)(suffix_len & 0x7);
        suffix_len >>= 3;
 
-       if (in.len < suffix_len)
+       if (in.len < suffix_len ||
+           prefix_len > last_key->len)
                return -1;
 
-       strbuf_reset(key);
-       strbuf_add(key, last_key.buf, prefix_len);
-       strbuf_add(key, in.buf, suffix_len);
+       strbuf_setlen(last_key, prefix_len);
+       strbuf_add(last_key, in.buf, suffix_len);
        string_view_consume(&in, suffix_len);
 
        return start_len - in.len;
@@ -205,14 +204,26 @@ static void reftable_ref_record_copy_from(void *rec, const void *src_rec,
 {
        struct reftable_ref_record *ref = rec;
        const struct reftable_ref_record *src = src_rec;
+       char *refname = NULL;
+       size_t refname_cap = 0;
+
        assert(hash_size > 0);
 
-       /* This is simple and correct, but we could probably reuse the hash
-        * fields. */
+       SWAP(refname, ref->refname);
+       SWAP(refname_cap, ref->refname_cap);
        reftable_ref_record_release(ref);
+       SWAP(ref->refname, refname);
+       SWAP(ref->refname_cap, refname_cap);
+
        if (src->refname) {
-               ref->refname = xstrdup(src->refname);
+               size_t refname_len = strlen(src->refname);
+
+               REFTABLE_ALLOC_GROW(ref->refname, refname_len + 1,
+                                   ref->refname_cap);
+               memcpy(ref->refname, src->refname, refname_len);
+               ref->refname[refname_len] = 0;
        }
+
        ref->update_index = src->update_index;
        ref->value_type = src->value_type;
        switch (src->value_type) {
@@ -368,16 +379,24 @@ static int reftable_ref_record_decode(void *rec, struct strbuf key,
        struct reftable_ref_record *r = rec;
        struct string_view start = in;
        uint64_t update_index = 0;
-       int n = get_var_int(&update_index, &in);
+       const char *refname = NULL;
+       size_t refname_cap = 0;
+       int n;
+
+       assert(hash_size > 0);
+
+       n = get_var_int(&update_index, &in);
        if (n < 0)
                return n;
        string_view_consume(&in, n);
 
+       SWAP(refname, r->refname);
+       SWAP(refname_cap, r->refname_cap);
        reftable_ref_record_release(r);
+       SWAP(r->refname, refname);
+       SWAP(r->refname_cap, refname_cap);
 
-       assert(hash_size > 0);
-
-       r->refname = reftable_malloc(key.len + 1);
+       REFTABLE_ALLOC_GROW(r->refname, key.len + 1, r->refname_cap);
        memcpy(r->refname, key.buf, key.len);
        r->refname[key.len] = 0;
 
@@ -1157,11 +1176,6 @@ void reftable_record_key(struct reftable_record *rec, struct strbuf *dest)
        reftable_record_vtable(rec)->key(reftable_record_data(rec), dest);
 }
 
-uint8_t reftable_record_type(struct reftable_record *rec)
-{
-       return rec->type;
-}
-
 int reftable_record_encode(struct reftable_record *rec, struct string_view dest,
                           int hash_size)
 {
@@ -1283,12 +1297,6 @@ int reftable_log_record_is_deletion(const struct reftable_log_record *log)
        return (log->value_type == REFTABLE_LOG_DELETION);
 }
 
-void string_view_consume(struct string_view *s, int n)
-{
-       s->buf += n;
-       s->len -= n;
-}
-
 static void *reftable_record_data(struct reftable_record *rec)
 {
        switch (rec->type) {
index a05e2be17971d5e26f27871159cda293aa2640ad..5e8304e05284c7bbc2f5b2940904023ae032f15a 100644 (file)
@@ -25,7 +25,11 @@ struct string_view {
 };
 
 /* Advance `s.buf` by `n`, and decrease length. */
-void string_view_consume(struct string_view *s, int n);
+static inline void string_view_consume(struct string_view *s, int n)
+{
+       s->buf += n;
+       s->len -= n;
+}
 
 /* utilities for de/encoding varints */
 
@@ -81,9 +85,12 @@ int reftable_encode_key(int *is_restart, struct string_view dest,
                        struct strbuf prev_key, struct strbuf key,
                        uint8_t extra);
 
-/* Decode into `key` and `extra` from `in` */
-int reftable_decode_key(struct strbuf *key, uint8_t *extra,
-                       struct strbuf last_key, struct string_view in);
+/*
+ * Decode into `last_key` and `extra` from `in`. `last_key` is expected to
+ * contain the decoded key of the preceding record, if any.
+ */
+int reftable_decode_key(struct strbuf *last_key, uint8_t *extra,
+                       struct string_view in);
 
 /* reftable_index_record are used internally to speed up lookups. */
 struct reftable_index_record {
@@ -124,7 +131,6 @@ int reftable_record_cmp(struct reftable_record *a, struct reftable_record *b);
 int reftable_record_equal(struct reftable_record *a, struct reftable_record *b, int hash_size);
 void reftable_record_print(struct reftable_record *rec, int hash_size);
 void reftable_record_key(struct reftable_record *rec, struct strbuf *dest);
-uint8_t reftable_record_type(struct reftable_record *rec);
 void reftable_record_copy_from(struct reftable_record *rec,
                               struct reftable_record *src, int hash_size);
 uint8_t reftable_record_val_type(struct reftable_record *rec);
@@ -135,6 +141,11 @@ int reftable_record_decode(struct reftable_record *rec, struct strbuf key,
                           int hash_size);
 int reftable_record_is_deletion(struct reftable_record *rec);
 
+static inline uint8_t reftable_record_type(struct reftable_record *rec)
+{
+       return rec->type;
+}
+
 /* frees and zeroes out the embedded record */
 void reftable_record_release(struct reftable_record *rec);
 
index a86cff552661ba0f00be32200bf8b0c800a9fa87..89209894d84395ce9ae476ba461fea04100c05a6 100644 (file)
@@ -295,7 +295,8 @@ static void test_key_roundtrip(void)
        EXPECT(!restart);
        EXPECT(n > 0);
 
-       m = reftable_decode_key(&roundtrip, &rt_extra, last_key, dest);
+       strbuf_addstr(&roundtrip, "refs/heads/master");
+       m = reftable_decode_key(&roundtrip, &rt_extra, dest);
        EXPECT(n == m);
        EXPECT(0 == strbuf_cmp(&key, &roundtrip));
        EXPECT(rt_extra == extra);
index bb6e99acd3151ec75a7a2dc60e930064b7322e60..e657001d42fc9e640052092c59db17121e60cf12 100644 (file)
@@ -22,6 +22,7 @@ https://developers.google.com/open-source/licenses/bsd
 /* reftable_ref_record holds a ref database entry target_value */
 struct reftable_ref_record {
        char *refname; /* Name of the ref, malloced. */
+       size_t refname_cap;
        uint64_t update_index; /* Logical timestamp at which this value is
                                * written */