refs/reftable: don't recompute committer ident

In order to write reflog entries we need to compute the committer's
identity as it gets encoded in the log record itself. The reftable
backend does this via `git_committer_info()` and `split_ident_line()` in
`fill_reftable_log_record()`, which use the Git config as well as
environment variables to figure out the identity.

While most callers would only call `fill_reftable_log_record()` once or
twice, `write_transaction_table()` will call it as many times as there
are queued ref updates. This can be quite a waste of effort when writing
many refs with reflog entries in a single transaction.

Refactor the code to pre-compute the committer information. This results
in a small speedup when writing 100000 refs in a single transaction:

  Benchmark 1: update-ref: create many refs (HEAD~)
    Time (mean ± σ):      2.895 s ±  0.020 s    [User: 1.516 s, System: 1.374 s]
    Range (min … max):    2.868 s …  2.983 s    100 runs

  Benchmark 2: update-ref: create many refs (HEAD)
    Time (mean ± σ):      2.845 s ±  0.017 s    [User: 1.461 s, System: 1.379 s]
    Range (min … max):    2.803 s …  2.913 s    100 runs

  Summary
    update-ref: create many refs (HEAD) ran
      1.02 ± 0.01 times faster than update-ref: create many refs (HEAD~)

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

diff --git a/refs/reftable-backend.c b/refs/reftable-backend.c
index 8a54b0d8b23735ee47d0124fd1020737eb7ecd9b..a5ef36ffa9cd53d571846072317d2bb34bc360be 100644 (file)
--- a/refs/reftable-backend.c
+++ b/refs/reftable-backend.c
@@ -171,32 +171,30 @@ static int should_write_log(struct ref_store *refs, const char *refname)
        }
 }
 
-static void fill_reftable_log_record(struct reftable_log_record *log)
+static void fill_reftable_log_record(struct reftable_log_record *log, const struct ident_split *split)
 {
-       const char *info = git_committer_info(0);
-       struct ident_split split = {0};
+       const char *tz_begin;
        int sign = 1;
 
-       if (split_ident_line(&split, info, strlen(info)))
-               BUG("failed splitting committer info");
-
        reftable_log_record_release(log);
        log->value_type = REFTABLE_LOG_UPDATE;
        log->value.update.name =
-               xstrndup(split.name_begin, split.name_end - split.name_begin);
+               xstrndup(split->name_begin, split->name_end - split->name_begin);
        log->value.update.email =
-               xstrndup(split.mail_begin, split.mail_end - split.mail_begin);
-       log->value.update.time = atol(split.date_begin);
-       if (*split.tz_begin == '-') {
+               xstrndup(split->mail_begin, split->mail_end - split->mail_begin);
+       log->value.update.time = atol(split->date_begin);
+
+       tz_begin = split->tz_begin;
+       if (*tz_begin == '-') {
                sign = -1;
-               split.tz_begin++;
+               tz_begin++;
        }
-       if (*split.tz_begin == '+') {
+       if (*tz_begin == '+') {
                sign = 1;
-               split.tz_begin++;
+               tz_begin++;
        }
 
-       log->value.update.tz_offset = sign * atoi(split.tz_begin);
+       log->value.update.tz_offset = sign * atoi(tz_begin);
 }
 
 static int read_ref_without_reload(struct reftable_stack *stack,
@@ -1018,9 +1016,15 @@ static int write_transaction_table(struct reftable_writer *writer, void *cb_data
                reftable_stack_merged_table(arg->stack);
        uint64_t ts = reftable_stack_next_update_index(arg->stack);
        struct reftable_log_record *logs = NULL;
+       struct ident_split committer_ident = {0};
        size_t logs_nr = 0, logs_alloc = 0, i;
+       const char *committer_info;
        int ret = 0;
 
+       committer_info = git_committer_info(0);
+       if (split_ident_line(&committer_ident, committer_info, strlen(committer_info)))
+               BUG("failed splitting committer info");
+
        QSORT(arg->updates, arg->updates_nr, transaction_update_cmp);
 
        reftable_writer_set_limits(writer, ts, ts);
@@ -1086,7 +1090,7 @@ static int write_transaction_table(struct reftable_writer *writer, void *cb_data
                        log = &logs[logs_nr++];
                        memset(log, 0, sizeof(*log));
 
-                       fill_reftable_log_record(log);
+                       fill_reftable_log_record(log, &committer_ident);
                        log->update_index = ts;
                        log->refname = xstrdup(u->refname);
                        memcpy(log->value.update.new_hash, u->new_oid.hash, GIT_MAX_RAWSZ);
@@ -1233,9 +1237,11 @@ static int write_create_symref_table(struct reftable_writer *writer, void *cb_da
                .value.symref = (char *)create->target,
                .update_index = ts,
        };
+       struct ident_split committer_ident = {0};
        struct reftable_log_record log = {0};
        struct object_id new_oid;
        struct object_id old_oid;
+       const char *committer_info;
        int ret;
 
        reftable_writer_set_limits(writer, ts, ts);
@@ -1263,7 +1269,11 @@ static int write_create_symref_table(struct reftable_writer *writer, void *cb_da
            !should_write_log(&create->refs->base, create->refname))
                return 0;
 
-       fill_reftable_log_record(&log);
+       committer_info = git_committer_info(0);
+       if (split_ident_line(&committer_ident, committer_info, strlen(committer_info)))
+               BUG("failed splitting committer info");
+
+       fill_reftable_log_record(&log, &committer_ident);
        log.refname = xstrdup(create->refname);
        log.update_index = ts;
        log.value.update.message = xstrndup(create->logmsg,
@@ -1339,10 +1349,16 @@ static int write_copy_table(struct reftable_writer *writer, void *cb_data)
        struct reftable_log_record old_log = {0}, *logs = NULL;
        struct reftable_iterator it = {0};
        struct string_list skip = STRING_LIST_INIT_NODUP;
+       struct ident_split committer_ident = {0};
        struct strbuf errbuf = STRBUF_INIT;
        size_t logs_nr = 0, logs_alloc = 0, i;
+       const char *committer_info;
        int ret;
 
+       committer_info = git_committer_info(0);
+       if (split_ident_line(&committer_ident, committer_info, strlen(committer_info)))
+               BUG("failed splitting committer info");
+
        if (reftable_stack_read_ref(arg->stack, arg->oldname, &old_ref)) {
                ret = error(_("refname %s not found"), arg->oldname);
                goto done;
@@ -1417,7 +1433,7 @@ static int write_copy_table(struct reftable_writer *writer, void *cb_data)
 
                ALLOC_GROW(logs, logs_nr + 1, logs_alloc);
                memset(&logs[logs_nr], 0, sizeof(logs[logs_nr]));
-               fill_reftable_log_record(&logs[logs_nr]);
+               fill_reftable_log_record(&logs[logs_nr], &committer_ident);
                logs[logs_nr].refname = (char *)arg->newname;
                logs[logs_nr].update_index = deletion_ts;
                logs[logs_nr].value.update.message =
@@ -1449,7 +1465,7 @@ static int write_copy_table(struct reftable_writer *writer, void *cb_data)
         */
        ALLOC_GROW(logs, logs_nr + 1, logs_alloc);
        memset(&logs[logs_nr], 0, sizeof(logs[logs_nr]));
-       fill_reftable_log_record(&logs[logs_nr]);
+       fill_reftable_log_record(&logs[logs_nr], &committer_ident);
        logs[logs_nr].refname = (char *)arg->newname;
        logs[logs_nr].update_index = creation_ts;
        logs[logs_nr].value.update.message =