summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorElijah Newren <newren@gmail.com>2021-07-30 11:47:43 (GMT)
committerJunio C Hamano <gitster@pobox.com>2021-07-30 16:01:19 (GMT)
commit092e5115d1312c0b30348404d3f5b2bb02909342 (patch)
tree8e455e4609b8c9d9fa8221883000839c5614c6a4
parentf239fff4c183b231134115bc3b38b4f8e3982d3e (diff)
downloadgit-092e5115d1312c0b30348404d3f5b2bb02909342.zip
git-092e5115d1312c0b30348404d3f5b2bb02909342.tar.gz
git-092e5115d1312c0b30348404d3f5b2bb02909342.tar.bz2
merge-ort: reuse path strings in pool_alloc_filespec
pool_alloc_filespec() was written so that the code when pool != NULL mimicked the code from alloc_filespec(), which including allocating enough extra space for the path and then copying it. However, the path passed to pool_alloc_filespec() is always going to already be in the same memory pool, so we may as well reuse it instead of copying it. For the testcases mentioned in commit 557ac0350d ("merge-ort: begin performance work; instrument with trace2_region_* calls", 2020-10-28), this change improves the performance as follows: Before After no-renames: 198.5 ms ± 3.4 ms 198.3 ms ± 2.9 ms mega-renames: 679.1 ms ± 5.6 ms 661.8 ms ± 5.9 ms just-one-mega: 271.9 ms ± 2.8 ms 264.6 ms ± 2.5 ms Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
-rw-r--r--merge-ort.c29
1 files changed, 22 insertions, 7 deletions
diff --git a/merge-ort.c b/merge-ort.c
index f4f0a3d..86ab8f6 100644
--- a/merge-ort.c
+++ b/merge-ort.c
@@ -694,17 +694,13 @@ static struct diff_filespec *pool_alloc_filespec(struct mem_pool *pool,
const char *path)
{
struct diff_filespec *spec;
- size_t len;
if (!pool)
return alloc_filespec(path);
- /* Same code as alloc_filespec, except allocate from pool */
- len = strlen(path);
-
- spec = mem_pool_calloc(pool, 1, st_add3(sizeof(*spec), len, 1));
- memcpy(spec+1, path, len);
- spec->path = (void*)(spec+1);
+ /* Similar to alloc_filespec, but allocate from pool and reuse path */
+ spec = mem_pool_calloc(pool, 1, sizeof(*spec));
+ spec->path = (char*)path; /* spec won't modify it */
spec->count = 1;
spec->is_binary = -1;
@@ -2904,6 +2900,25 @@ static void use_cached_pairs(struct merge_options *opt,
const char *new_name = entry->value;
if (!new_name)
new_name = old_name;
+ if (pool) {
+ /*
+ * cached_pairs has _copies* of old_name and new_name,
+ * because it has to persist across merges. When
+ * pool != NULL
+ * pool_alloc_filespec() will just re-use the existing
+ * filenames, which will also get re-used by
+ * opt->priv->paths if they become renames, and then
+ * get freed at the end of the merge, leaving the copy
+ * in cached_pairs dangling. Avoid this by making a
+ * copy here.
+ *
+ * When pool == NULL, pool_alloc_filespec() calls
+ * alloc_filespec(), which makes a copy; we don't want
+ * to add another.
+ */
+ old_name = mem_pool_strdup(pool, old_name);
+ new_name = mem_pool_strdup(pool, new_name);
+ }
/* We don't care about oid/mode, only filenames and status */
one = pool_alloc_filespec(pool, old_name);