Re: [PATCH 0/5] Skip 'cache_tree_update()' when 'prime_cache_tree()' is called immediate after

[Derrick Stolee <derrickstolee@github.com>]

On 11/8/2022 5:44 PM, Victoria Dye via GitGitGadget wrote:
> Following up on a discussion [1] around cache tree refreshes in 'git reset',
> this series updates callers of 'unpack_trees()' to skip its internal
> invocation of 'cache_tree_update()' when 'prime_cache_tree()' is called
> immediately after 'unpack_trees()'. 'cache_tree_update()' can be an
> expensive operation, and it is redundant when 'prime_cache_tree()' clears
> and rebuilds the cache tree from scratch immediately after.
> 
> The first patch adds a test directly comparing the execution time of
> 'prime_cache_tree()' with that of 'cache_tree_update()'. The results show
> that on a fully-valid cache tree, they perform the same, but on a
> fully-invalid cache tree, 'prime_cache_tree()' is multiple times faster
> (although both are so fast that the total execution time of 100 invocations
> is needed to compare the results in the default perf repo).

One thing I found interesting is how you needed 200 iterations to show
a meaningful change in this test script, but in the case of 'git reset'
we can see sizeable improvements even with a single iteration.

Is there something about this test that is artificially speeding up
these iterations? Perhaps the index has up-to-date filesystem information
that allows these methods to avoid filesystem interactions that are
necessary in the 'git reset' case?
 
> The second patch introduces the 'skip_cache_tree_update' option for
> 'unpack_trees()', but does not use it yet.
> 
> The remaining three patches update callers that make the aforementioned
> redundant cache tree updates. The performance impact on these callers ranges
> from "negligible" (in 'rebase') to "substantial" (in 'read-tree') - more
> details can be found in the commit messages of the patch associated with the
> affected code path.

I found these patches well motivated and the code change to be so
unobtrusive that the benefits are well worth the new options.

Thanks,
-Stolee