Re: [PATCH 0/1] blame: remove unnecessary use of get_commit_info()

From: Derrick Stolee <stolee@gmail.com>
Date: Feb 16, 2021, 18:35 UTC
Message-ID: <ce29b876-a5dd-76e9-bf79-5b917249b508@gmail.com>
In-Reply-To: <20210216163151.76307-1-rafaeloliveira.cs@gmail.com>

On 2/16/2021 11:31 AM, Rafael Silva wrote:

Show 14 quoted lines

> Running Git PERF suite in linux.git, I've got a subtle performance
> improvement for some runs:
> 
> 	# git.328c109303 - compiled git from commit 328c109303
> 	# git.blame-patched - compiled git from commit 328c109303 + this patch
>         Test                                          git.328c109303    git.blame-patched
>         -------------------------------------------------------------------------------------
>         blame --color-by-age kernel/fork.c            1.96(1.81+0.15)   1.95(1.80+0.14) -0.5%
>         blame --color-by-age kernel/sys.c             1.67(1.53+0.13)   1.66(1.52+0.14) -0.6%
>         blame --color-by-age mm/slab.c                2.16(2.00+0.16)   2.15(1.99+0.15) -0.5%
>         blame --color-by-age lib/packing.c            0.20(0.14+0.05)   0.20(0.14+0.05) +0.0%
>         blame --color-by-age drivers/cdrom/cdrom.c    1.62(1.46+0.15)   1.62(1.46+0.15) +0.0%
>         blame --color-by-age crypto/crypto_engine.c   0.37(0.29+0.06)   0.36(0.28+0.06) -2.7%
>         blame --color-by-age net/ipv4/ip_forward.c    1.49(1.35+0.13)   1.48(1.34+0.13) -0.7%

Have you updated the commit-graph with changed-path Bloom filters in your copy of linux.git before running the perf tests? You might get smaller numbers overall (both sides) but make the difference for this patch be more pronounced:

	git commit-graph write --reachable --changed-paths

Show 15 quoted lines

> To dig a little deeper, I enabled the Git's trace2 API to record every
> call to the determine_line_heat() function:
> 
>         ...
> +       trace2_region_enter("blame", "determine_line_heat", the_repository);
>         determine_line_heat(ent, &default_color);
> +       trace2_region_enter("blame", "determine_line_heat", the_repository);
>         ...
> 
> Then, running `blame` for "kernel/fork.c` and _summing_ all the execution
> time for every call (around 1.3k calls) resulted in 2.6x faster execution
> (best out 3):
> 
> 	git built from 328c109303 (The eighth batch, 2021-02-12) = 42ms
> 	git built from 328c109303 + this patch                   = 16ms

This is a good way to demonstrate the change. Definitely worthwhile for demonstrating the value of the patch. I'll second Taylor's request that this performance data goes in the commit message so we can see the details in the future.

Thanks, -Stolee