name-rev: use mem_pool_strfmt()

1c56fc2084 (name-rev: pre-size buffer in get_parent_name(), 2020-02-04) got a big performance boost in an unusual repository by calculating the name length in advance. This is a bit awkward, as it references the name components twice. Use a memory pool to store the strings for the struct rev_name member tip_name. Using mem_pool_strfmt() allows efficient allocation without explicit size calculation. This simplifies the formatting part of the code without giving up performance: Benchmark 1: ./git_2.44.0 -C ../chromium/src name-rev --all Time (mean ± σ): 1.231 s ± 0.013 s [User: 1.082 s, System: 0.136 s] Range (min … max): 1.214 s … 1.252 s 10 runs Benchmark 2: ./git -C ../chromium/src name-rev --all Time (mean ± σ): 1.220 s ± 0.020 s [User: 1.083 s, System: 0.130 s] Range (min … max): 1.197 s … 1.254 s 10 runs Don't bother discarding the memory pool just before exiting. The effort for that would be very low, but actually measurable in the above example, with no benefit to users. At least UNLEAK it to calm down leak checkers. This addresses the leaks that 45a14f578e (Revert "name-rev: release unused name strings", 2022-04-22) brought back. Signed-off-by: René Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>

René Scharfe committed Feb 25, 2024 at 12:39 UTC f39addd0d9d75a073847ed4311079a499dd33f35
1 file changed +20 -19
builtin/name-rev.c
+20 -19
@@ -15,6 +15,7 @@
15 #include "commit-slab.h"
16 #include "commit-graph.h"
17 #include "wildmatch.h"
18 +#include "mem-pool.h"
19
20 /*
21 * One day. See the 'name a rev shortly after epoch' test in t6120 when
@@ -155,30 +156,25 @@ static struct rev_name *create_or_update_name(struct commit *commit,
156 return name;
157 }
158
158 -static char *get_parent_name(const struct rev_name *name, int parent_number)
159 +static char *get_parent_name(const struct rev_name *name, int parent_number,
160 + struct mem_pool *string_pool)
161 {
160 - struct strbuf sb = STRBUF_INIT;
162 size_t len;
163
164 strip_suffix(name->tip_name, "^0", &len);
165 if (name->generation > 0) {
165 - strbuf_grow(&sb, len +
166 - 1 + decimal_width(name->generation) +
167 - 1 + decimal_width(parent_number));
168 - strbuf_addf(&sb, "%.*s~%d^%d", (int)len, name->tip_name,
169 - name->generation, parent_number);
166 + return mem_pool_strfmt(string_pool, "%.*s~%d^%d",
167 + (int)len, name->tip_name,
168 + name->generation, parent_number);
169 } else {
171 - strbuf_grow(&sb, len +
172 - 1 + decimal_width(parent_number));
173 - strbuf_addf(&sb, "%.*s^%d", (int)len, name->tip_name,
174 - parent_number);
170 + return mem_pool_strfmt(string_pool, "%.*s^%d",
171 + (int)len, name->tip_name, parent_number);
172 }
176 - return strbuf_detach(&sb, NULL);
173 }
174
175 static void name_rev(struct commit *start_commit,
176 const char *tip_name, timestamp_t taggerdate,
181 - int from_tag, int deref)
177 + int from_tag, int deref, struct mem_pool *string_pool)
178 {
179 struct prio_queue queue;
180 struct commit *commit;
@@ -195,9 +191,10 @@ static void name_rev(struct commit *start_commit,
191 if (!start_name)
192 return;
193 if (deref)
198 - start_name->tip_name = xstrfmt("%s^0", tip_name);
194 + start_name->tip_name = mem_pool_strfmt(string_pool, "%s^0",
195 + tip_name);
196 else
200 - start_name->tip_name = xstrdup(tip_name);
197 + start_name->tip_name = mem_pool_strdup(string_pool, tip_name);
198
199 memset(&queue, 0, sizeof(queue)); /* Use the prio_queue as LIFO */
200 prio_queue_put(&queue, start_commit);
@@ -235,7 +232,8 @@ static void name_rev(struct commit *start_commit,
232 if (parent_number > 1)
233 parent_name->tip_name =
234 get_parent_name(name,
238 - parent_number);
235 + parent_number,
236 + string_pool);
237 else
238 parent_name->tip_name = name->tip_name;
239 ALLOC_GROW(parents_to_queue,
@@ -415,7 +413,7 @@ static int name_ref(const char *path, const struct object_id *oid,
413 return 0;
414 }
415
418 -static void name_tips(void)
416 +static void name_tips(struct mem_pool *string_pool)
417 {
418 int i;
419
@@ -428,7 +426,7 @@ static void name_tips(void)
426 struct tip_table_entry *e = &tip_table.table[i];
427 if (e->commit) {
428 name_rev(e->commit, e->refname, e->taggerdate,
431 - e->from_tag, e->deref);
429 + e->from_tag, e->deref, string_pool);
430 }
431 }
432 }
@@ -561,6 +559,7 @@ static void name_rev_line(char *p, struct name_ref_data *data)
559
560 int cmd_name_rev(int argc, const char **argv, const char *prefix)
561 {
562 + struct mem_pool string_pool;
563 struct object_array revs = OBJECT_ARRAY_INIT;
564 int all = 0, annotate_stdin = 0, transform_stdin = 0, allow_undefined = 1, always = 0, peel_tag = 0;
565 struct name_ref_data data = { 0, 0, STRING_LIST_INIT_NODUP, STRING_LIST_INIT_NODUP };
@@ -587,6 +586,7 @@ int cmd_name_rev(int argc, const char **argv, const char *prefix)
586 OPT_END(),
587 };
588
589 + mem_pool_init(&string_pool, 0);
590 init_commit_rev_name(&rev_names);
591 git_config(git_default_config, NULL);
592 argc = parse_options(argc, argv, prefix, opts, name_rev_usage, 0);
@@ -648,7 +648,7 @@ int cmd_name_rev(int argc, const char **argv, const char *prefix)
648 adjust_cutoff_timestamp_for_slop();
649
650 for_each_ref(name_ref, &data);
651 - name_tips();
651 + name_tips(&string_pool);
652
653 if (annotate_stdin) {
654 struct strbuf sb = STRBUF_INIT;
@@ -676,6 +676,7 @@ int cmd_name_rev(int argc, const char **argv, const char *prefix)
676 always, allow_undefined, data.name_only);
677 }
678
679 + UNLEAK(string_pool);
680 UNLEAK(revs);
681 return 0;
682 }