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

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

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

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