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>

Patrick Steinhardt committed Apr 8, 2024 at 14:24 UTC 44afd85fbd4cf69a0f030df98b3626f82e51bb33
1 file changed +34 -18
refs/reftable-backend.c
+34 -18
@@ -171,32 +171,30 @@ static int should_write_log(struct ref_store *refs, const char *refname)
171 }
172 }
173
174 -static void fill_reftable_log_record(struct reftable_log_record *log)
174 +static void fill_reftable_log_record(struct reftable_log_record *log, const struct ident_split *split)
175 {
176 - const char *info = git_committer_info(0);
177 - struct ident_split split = {0};
176 + const char *tz_begin;
177 int sign = 1;
178
180 - if (split_ident_line(&split, info, strlen(info)))
181 - BUG("failed splitting committer info");
182 -
179 reftable_log_record_release(log);
180 log->value_type = REFTABLE_LOG_UPDATE;
181 log->value.update.name =
186 - xstrndup(split.name_begin, split.name_end - split.name_begin);
182 + xstrndup(split->name_begin, split->name_end - split->name_begin);
183 log->value.update.email =
188 - xstrndup(split.mail_begin, split.mail_end - split.mail_begin);
189 - log->value.update.time = atol(split.date_begin);
190 - if (*split.tz_begin == '-') {
184 + xstrndup(split->mail_begin, split->mail_end - split->mail_begin);
185 + log->value.update.time = atol(split->date_begin);
186 +
187 + tz_begin = split->tz_begin;
188 + if (*tz_begin == '-') {
189 sign = -1;
192 - split.tz_begin++;
190 + tz_begin++;
191 }
194 - if (*split.tz_begin == '+') {
192 + if (*tz_begin == '+') {
193 sign = 1;
196 - split.tz_begin++;
194 + tz_begin++;
195 }
196
199 - log->value.update.tz_offset = sign * atoi(split.tz_begin);
197 + log->value.update.tz_offset = sign * atoi(tz_begin);
198 }
199
200 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
1016 reftable_stack_merged_table(arg->stack);
1017 uint64_t ts = reftable_stack_next_update_index(arg->stack);
1018 struct reftable_log_record *logs = NULL;
1019 + struct ident_split committer_ident = {0};
1020 size_t logs_nr = 0, logs_alloc = 0, i;
1021 + const char *committer_info;
1022 int ret = 0;
1023
1024 + committer_info = git_committer_info(0);
1025 + if (split_ident_line(&committer_ident, committer_info, strlen(committer_info)))
1026 + BUG("failed splitting committer info");
1027 +
1028 QSORT(arg->updates, arg->updates_nr, transaction_update_cmp);
1029
1030 reftable_writer_set_limits(writer, ts, ts);
@@ -1086,7 +1090,7 @@ static int write_transaction_table(struct reftable_writer *writer, void *cb_data
1090 log = &logs[logs_nr++];
1091 memset(log, 0, sizeof(*log));
1092
1089 - fill_reftable_log_record(log);
1093 + fill_reftable_log_record(log, &committer_ident);
1094 log->update_index = ts;
1095 log->refname = xstrdup(u->refname);
1096 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
1237 .value.symref = (char *)create->target,
1238 .update_index = ts,
1239 };
1240 + struct ident_split committer_ident = {0};
1241 struct reftable_log_record log = {0};
1242 struct object_id new_oid;
1243 struct object_id old_oid;
1244 + const char *committer_info;
1245 int ret;
1246
1247 reftable_writer_set_limits(writer, ts, ts);
@@ -1263,7 +1269,11 @@ static int write_create_symref_table(struct reftable_writer *writer, void *cb_da
1269 !should_write_log(&create->refs->base, create->refname))
1270 return 0;
1271
1266 - fill_reftable_log_record(&log);
1272 + committer_info = git_committer_info(0);
1273 + if (split_ident_line(&committer_ident, committer_info, strlen(committer_info)))
1274 + BUG("failed splitting committer info");
1275 +
1276 + fill_reftable_log_record(&log, &committer_ident);
1277 log.refname = xstrdup(create->refname);
1278 log.update_index = ts;
1279 log.value.update.message = xstrndup(create->logmsg,
@@ -1339,10 +1349,16 @@ static int write_copy_table(struct reftable_writer *writer, void *cb_data)
1349 struct reftable_log_record old_log = {0}, *logs = NULL;
1350 struct reftable_iterator it = {0};
1351 struct string_list skip = STRING_LIST_INIT_NODUP;
1352 + struct ident_split committer_ident = {0};
1353 struct strbuf errbuf = STRBUF_INIT;
1354 size_t logs_nr = 0, logs_alloc = 0, i;
1355 + const char *committer_info;
1356 int ret;
1357
1358 + committer_info = git_committer_info(0);
1359 + if (split_ident_line(&committer_ident, committer_info, strlen(committer_info)))
1360 + BUG("failed splitting committer info");
1361 +
1362 if (reftable_stack_read_ref(arg->stack, arg->oldname, &old_ref)) {
1363 ret = error(_("refname %s not found"), arg->oldname);
1364 goto done;
@@ -1417,7 +1433,7 @@ static int write_copy_table(struct reftable_writer *writer, void *cb_data)
1433
1434 ALLOC_GROW(logs, logs_nr + 1, logs_alloc);
1435 memset(&logs[logs_nr], 0, sizeof(logs[logs_nr]));
1420 - fill_reftable_log_record(&logs[logs_nr]);
1436 + fill_reftable_log_record(&logs[logs_nr], &committer_ident);
1437 logs[logs_nr].refname = (char *)arg->newname;
1438 logs[logs_nr].update_index = deletion_ts;
1439 logs[logs_nr].value.update.message =
@@ -1449,7 +1465,7 @@ static int write_copy_table(struct reftable_writer *writer, void *cb_data)
1465 */
1466 ALLOC_GROW(logs, logs_nr + 1, logs_alloc);
1467 memset(&logs[logs_nr], 0, sizeof(logs[logs_nr]));
1452 - fill_reftable_log_record(&logs[logs_nr]);
1468 + fill_reftable_log_record(&logs[logs_nr], &committer_ident);
1469 logs[logs_nr].refname = (char *)arg->newname;
1470 logs[logs_nr].update_index = creation_ts;
1471 logs[logs_nr].value.update.message =