From: "Elijah Newren via GitGitGadget" <gitgitgadget@gmail.com>
To: git@vger.kernel.org
Cc: Jeff King <peff@peff.net>, Elijah Newren <newren@gmail.com>,
Elijah Newren <newren@gmail.com>
Subject: [PATCH 7/7] merge-ort: reuse path strings in pool_alloc_filespec
Date: Fri, 23 Jul 2021 12:54:57 +0000 [thread overview]
Message-ID: <17aa0a748494e72df1e0e2303ed650917e59a101.1627044898.git.gitgitgadget@gmail.com> (raw)
In-Reply-To: <pull.990.git.1627044897.gitgitgadget@gmail.com>
From: Elijah Newren <newren@gmail.com>
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>
---
merge-ort.c | 29 ++++++++++++++++++++++-------
1 file changed, 22 insertions(+), 7 deletions(-)
diff --git a/merge-ort.c b/merge-ort.c
index d29c7fe8a30..0fb942692a7 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);
--
gitgitgadget
next prev parent reply other threads:[~2021-07-23 12:57 UTC|newest]
Thread overview: 65+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-07-23 12:54 [PATCH 0/7] Final optimization batch (#15): use memory pools Elijah Newren via GitGitGadget
2021-07-23 12:54 ` [PATCH 1/7] diffcore-rename: use a mem_pool for exact rename detection's hashmap Elijah Newren via GitGitGadget
2021-07-23 21:59 ` Eric Sunshine
2021-07-23 22:03 ` Elijah Newren
2021-07-23 12:54 ` [PATCH 2/7] merge-ort: set up a memory pool Elijah Newren via GitGitGadget
2021-07-23 12:54 ` [PATCH 3/7] merge-ort: add pool_alloc, pool_calloc, and pool_strndup wrappers Elijah Newren via GitGitGadget
2021-07-23 22:07 ` Eric Sunshine
2021-07-26 14:36 ` Derrick Stolee
2021-07-28 22:49 ` Elijah Newren
2021-07-29 15:26 ` Jeff King
2021-07-30 2:27 ` Elijah Newren
2021-07-30 16:12 ` Jeff King
2021-07-23 12:54 ` [PATCH 4/7] merge-ort: switch our strmaps over to using memory pools Elijah Newren via GitGitGadget
2021-07-23 12:54 ` [PATCH 5/7] diffcore-rename, merge-ort: add wrapper functions for filepair alloc/dealloc Elijah Newren via GitGitGadget
2021-07-23 12:54 ` [PATCH 6/7] merge-ort: store filepairs and filespecs in our mem_pool Elijah Newren via GitGitGadget
2021-07-23 12:54 ` Elijah Newren via GitGitGadget [this message]
2021-07-26 14:44 ` [PATCH 0/7] Final optimization batch (#15): use memory pools Derrick Stolee
2021-07-28 22:52 ` Elijah Newren
2021-07-29 3:58 ` [PATCH v2 " Elijah Newren via GitGitGadget
2021-07-29 3:58 ` [PATCH v2 1/7] diffcore-rename: use a mem_pool for exact rename detection's hashmap Elijah Newren via GitGitGadget
2021-07-29 3:58 ` [PATCH v2 2/7] merge-ort: add pool_alloc, pool_calloc, and pool_strndup wrappers Elijah Newren via GitGitGadget
2021-07-29 3:58 ` [PATCH v2 3/7] merge-ort: set up a memory pool Elijah Newren via GitGitGadget
2021-07-29 3:58 ` [PATCH v2 4/7] merge-ort: switch our strmaps over to using memory pools Elijah Newren via GitGitGadget
2021-07-29 15:28 ` Jeff King
2021-07-29 18:37 ` Elijah Newren
2021-07-29 20:09 ` Jeff King
2021-07-30 2:30 ` Elijah Newren
2021-07-30 16:12 ` Jeff King
2021-07-30 13:30 ` Ævar Arnfjörð Bjarmason
2021-07-30 14:36 ` Elijah Newren
2021-07-30 16:23 ` Ævar Arnfjörð Bjarmason
2021-07-29 3:58 ` [PATCH v2 5/7] diffcore-rename, merge-ort: add wrapper functions for filepair alloc/dealloc Elijah Newren via GitGitGadget
2021-07-29 3:58 ` [PATCH v2 6/7] merge-ort: store filepairs and filespecs in our mem_pool Elijah Newren via GitGitGadget
2021-07-29 3:58 ` [PATCH v2 7/7] merge-ort: reuse path strings in pool_alloc_filespec Elijah Newren via GitGitGadget
2021-07-29 14:58 ` [PATCH v2 0/7] Final optimization batch (#15): use memory pools Derrick Stolee
2021-07-29 16:20 ` Jeff King
2021-07-29 16:23 ` Jeff King
2021-07-29 19:46 ` Junio C Hamano
2021-07-29 20:48 ` Junio C Hamano
2021-07-29 21:05 ` Elijah Newren
2021-07-29 20:46 ` Elijah Newren
2021-07-29 21:14 ` Jeff King
2021-07-30 11:47 ` [PATCH v3 0/9] " Elijah Newren via GitGitGadget
2021-07-30 11:47 ` [PATCH v3 1/9] merge-ort: rename str{map,intmap,set}_func() Elijah Newren via GitGitGadget
2021-07-30 11:47 ` [PATCH v3 2/9] diffcore-rename: use a mem_pool for exact rename detection's hashmap Elijah Newren via GitGitGadget
2021-07-30 11:47 ` [PATCH v3 3/9] merge-ort: add pool_alloc, pool_calloc, and pool_strndup wrappers Elijah Newren via GitGitGadget
2021-07-30 11:47 ` [PATCH v3 4/9] merge-ort: set up a memory pool Elijah Newren via GitGitGadget
2021-07-30 11:47 ` [PATCH v3 5/9] merge-ort: switch our strmaps over to using memory pools Elijah Newren via GitGitGadget
2021-07-30 11:47 ` [PATCH v3 6/9] diffcore-rename, merge-ort: add wrapper functions for filepair alloc/dealloc Elijah Newren via GitGitGadget
2021-07-30 11:47 ` [PATCH v3 7/9] merge-ort: store filepairs and filespecs in our mem_pool Elijah Newren via GitGitGadget
2021-07-30 11:47 ` [PATCH v3 8/9] merge-ort: reuse path strings in pool_alloc_filespec Elijah Newren via GitGitGadget
2021-07-30 11:47 ` [PATCH v3 9/9] merge-ort: remove compile-time ability to turn off usage of memory pools Elijah Newren via GitGitGadget
2021-07-30 16:24 ` Jeff King
2021-07-31 17:27 ` [PATCH v4 0/9] Final optimization batch (#15): use " Elijah Newren via GitGitGadget
2021-07-31 17:27 ` [PATCH v4 1/9] merge-ort: rename str{map,intmap,set}_func() Elijah Newren via GitGitGadget
2021-07-31 17:27 ` [PATCH v4 2/9] diffcore-rename: use a mem_pool for exact rename detection's hashmap Elijah Newren via GitGitGadget
2021-07-31 17:27 ` [PATCH v4 3/9] merge-ort: add pool_alloc, pool_calloc, and pool_strndup wrappers Elijah Newren via GitGitGadget
2021-07-31 17:27 ` [PATCH v4 4/9] merge-ort: set up a memory pool Elijah Newren via GitGitGadget
2021-07-31 17:27 ` [PATCH v4 5/9] merge-ort: switch our strmaps over to using memory pools Elijah Newren via GitGitGadget
2021-07-31 17:27 ` [PATCH v4 6/9] diffcore-rename, merge-ort: add wrapper functions for filepair alloc/dealloc Elijah Newren via GitGitGadget
2021-07-31 17:27 ` [PATCH v4 7/9] merge-ort: store filepairs and filespecs in our mem_pool Elijah Newren via GitGitGadget
2021-07-31 17:27 ` [PATCH v4 8/9] merge-ort: reuse path strings in pool_alloc_filespec Elijah Newren via GitGitGadget
2021-07-31 17:27 ` [PATCH v4 9/9] merge-ort: remove compile-time ability to turn off usage of memory pools Elijah Newren via GitGitGadget
2021-08-02 15:27 ` [PATCH v4 0/9] Final optimization batch (#15): use " Derrick Stolee
2021-08-03 15:45 ` Jeff King
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: http://vger.kernel.org/majordomo-info.html
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=17aa0a748494e72df1e0e2303ed650917e59a101.1627044898.git.gitgitgadget@gmail.com \
--to=gitgitgadget@gmail.com \
--cc=git@vger.kernel.org \
--cc=newren@gmail.com \
--cc=peff@peff.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).