Junio C Hamano <gitster@xxxxxxxxx> writes:
> Antoine Pelisse <apelisse@xxxxxxxxx> writes:
>
>> Have you been able to measure a speed increase due to less copies ?
>
> No.
>
> This topic was not strictly my itch, but I did the rewrite because I
> couldn't stand staring at that *_extended() function ;-)
Now I have. The overall numbers are larger primarily because the
kernel history has more commits than back when I did the previous
commit (thanks for reminding me of that one, by the way), but the
basic trend is the same.
The patch with updated numbers in the log is attached.
I am not absolutely sure about the correctness of the third patch in
the series; a review on the logic that uses the insertion point
search to implement the prefix match is very much appreciated.
Thanks.
-- >8 --
Subject: [PATCH] log --use-mailmap: optimize for cases without --author/--committer search
When we taught the commit_match() mechanism to pay attention to the
new --use-mailmap option, we started to unconditionally copy the
commit object to a temporary buffer, just in case we need the author
and committer lines updated via the mailmap mechanism, and rewrite
author and committer using the mailmap.
It turns out that this has a rather unpleasant performance
implications. In the linux kernel repository, running
$ git log --author='Junio C Hamano' --pretty=short >/dev/null
under /usr/bin/time, with and without --use-mailmap (the .mailmap
file is 118 entries long, the particular author does not appear in
it), cost (with warm cache):
[without --use-mailmap]
5.42user 0.26system 0:05.70elapsed 99%CPU (0avgtext+0avgdata 2005936maxresident)k
0inputs+0outputs (0major+137669minor)pagefaults 0swaps
[with --use-mailmap]
6.47user 0.30system 0:06.78elapsed 99%CPU (0avgtext+0avgdata 2006288maxresident)k
0inputs+0outputs (0major+137692minor)pagefaults 0swaps
which incurs about 20% overhead. The command is doing extra work,
so the extra cost may be justified.
But it is inexcusable to pay the cost when we do not need
author/committer match. In the same repository,
$ git log --grep='fix menuconfig on debian lenny' --pretty=short >/dev/null
shows very similar numbers as the above:
[without --use-mailmap]
5.32user 0.30system 0:05.63elapsed 99%CPU (0avgtext+0avgdata 2005984maxresident)k
0inputs+0outputs (0major+137672minor)pagefaults 0swaps
[with --use-mailmap]
6.64user 0.24system 0:06.89elapsed 99%CPU (0avgtext+0avgdata 2006320maxresident)k
0inputs+0outputs (0major+137694minor)pagefaults 0swaps
The latter case is an unnecessary performance regression. We may
want to _show_ the result with mailmap applied, but we do not have
to copy and rewrite the author/committer of all commits we try to
match if we do not query for these fields.
Trivially optimize this performace regression by limiting the
rewrites for only when we are matching with author/committer fields.
Signed-off-by: Junio C Hamano <gitster@xxxxxxxxx>
---
revision.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/revision.c b/revision.c
index 2cce85a..d7562ee 100644
--- a/revision.c
+++ b/revision.c
@@ -2283,7 +2283,7 @@ static int commit_match(struct commit *commit, struct rev_info *opt)
if (buf.len)
strbuf_addstr(&buf, commit->buffer);
- if (opt->mailmap) {
+ if (opt->grep_filter.header_list && opt->mailmap) {
if (!buf.len)
strbuf_addstr(&buf, commit->buffer);
--
1.8.1.304.gf036638
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html