reftable/record: avoid copying author info

Each reflog entry contains information regarding the authorship of who
has made the change. This authorship information is not the same as that
of any of the commits that the reflog entry references, but instead
corresponds to the local user that has executed the command. Thus, it is
almost always the case that all reflog entries have the same author.

We can make use of this fact when decoding reftable records: instead of
freeing and then reallocating the authorship information of log records,
we can special-case when the next record during an iteration has the
exact same authorship as the preceding record. If so, then there is no
need to reallocate the respective fields.

This change results in two allocations less per log record that we're
iterating over in the most common case. Before:

    HEAP SUMMARY:
        in use at exit: 13,473 bytes in 122 blocks
      total heap usage: 6,068,489 allocs, 6,068,367 frees, 361,011,822 bytes allocated

After:

    HEAP SUMMARY:
        in use at exit: 13,473 bytes in 122 blocks
      total heap usage: 4,068,487 allocs, 4,068,365 frees, 332,011,793 bytes allocated

An alternative would be to store the capacity of both name and email and
then use `REFTABLE_ALLOC_GROW()` to conditionally reallocate the array.
But reftable records are copied around quite a lot, and thus we need to
be a bit mindful of the overall record size. Furthermore, a memory
comparison should also be more efficient than having to copy over memory
even if we wouldn't have to allocate a new array every time.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/reftable/record.c b/reftable/record.c
index 8d2cd6b081e5e38556bd1b9d81f7a3840de1f9bb..d816de6d9333afc8471fb4526893859d6cc11ac3 100644 (file)
--- a/reftable/record.c
+++ b/reftable/record.c
@@ -896,10 +896,19 @@ static int reftable_log_record_decode(void *rec, struct strbuf key,
                goto done;
        string_view_consume(&in, n);
 
-       r->value.update.name =
-               reftable_realloc(r->value.update.name, dest.len + 1);
-       memcpy(r->value.update.name, dest.buf, dest.len);
-       r->value.update.name[dest.len] = 0;
+       /*
+        * In almost all cases we can expect the reflog name to not change for
+        * reflog entries as they are tied to the local identity, not to the
+        * target commits. As an optimization for this common case we can thus
+        * skip copying over the name in case it's accurate already.
+        */
+       if (!r->value.update.name ||
+           strcmp(r->value.update.name, dest.buf)) {
+               r->value.update.name =
+                       reftable_realloc(r->value.update.name, dest.len + 1);
+               memcpy(r->value.update.name, dest.buf, dest.len);
+               r->value.update.name[dest.len] = 0;
+       }
 
        strbuf_reset(&dest);
        n = decode_string(&dest, in);
@@ -907,10 +916,14 @@ static int reftable_log_record_decode(void *rec, struct strbuf key,
                goto done;
        string_view_consume(&in, n);
 
-       r->value.update.email =
-               reftable_realloc(r->value.update.email, dest.len + 1);
-       memcpy(r->value.update.email, dest.buf, dest.len);
-       r->value.update.email[dest.len] = 0;
+       /* Same as above, but for the reflog email. */
+       if (!r->value.update.email ||
+           strcmp(r->value.update.email, dest.buf)) {
+               r->value.update.email =
+                       reftable_realloc(r->value.update.email, dest.len + 1);
+               memcpy(r->value.update.email, dest.buf, dest.len);
+               r->value.update.email[dest.len] = 0;
+       }
 
        ts = 0;
        n = get_var_int(&ts, &in);