mirrors/git
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

merge-ort: reuse path strings in pool_alloc_filespec

Browse Source 
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>
This commit is contained in:
Elijah Newren
2021-07-30 11:47:43 +00:00
committed by Junio C Hamano
parent f239fff4c1
commit 092e5115d1
1 changed files with 22 additions and 7 deletions
Download Patch File Download Diff File Expand all files Collapse all files

29
merge-ort.c
View File

@ -694,17 +694,13 @@ static struct diff_filespec *pool_alloc_filespec(struct mem_pool *pool,
const char *path) const char *path)
{ {
struct diff_filespec *spec; struct diff_filespec *spec;
size_t len;
if (!pool) if (!pool)
return alloc_filespec(path); return alloc_filespec(path);
/* Same code as alloc_filespec, except allocate from pool */ /* Similar to alloc_filespec, but allocate from pool and reuse path */
len = strlen(path); spec = mem_pool_calloc(pool, 1, sizeof(*spec));
spec->path = (char*)path; /* spec won't modify it */
spec = mem_pool_calloc(pool, 1, st_add3(sizeof(*spec), len, 1));
memcpy(spec+1, path, len);
spec->path = (void*)(spec+1);
spec->count = 1; spec->count = 1;
spec->is_binary = -1; spec->is_binary = -1;
@ -2904,6 +2900,25 @@ static void use_cached_pairs(struct merge_options *opt,
const char *new_name = entry->value; const char *new_name = entry->value;
if (!new_name) if (!new_name)
new_name = old_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 */ /* We don't care about oid/mode, only filenames and status */
one = pool_alloc_filespec(pool, old_name); one = pool_alloc_filespec(pool, old_name);