On 12/30/2020 2:48 PM, Elijah Newren wrote:
> On Wed, Dec 30, 2020 at 11:26 AM Derrick Stolee via GitGitGadget
> <gitgitgadget@xxxxxxxxx> wrote:
>>
>> From: Derrick Stolee <dstolee@xxxxxxxxxxxxx>
>>
>> Commands such as "git reset --hard" rebuild the in-memory representation
>> of the cached tree index extension by parsing tree objects starting at a
>> known root tree. The performance of this operation can vary widely
>> depending on the width and depth of the repository's working directory
>> structure. Measure the time in this operation using trace2 regions in
>> prime_cache_tree().
>>
>> Signed-off-by: Derrick Stolee <dstolee@xxxxxxxxxxxxx>
>> ---
>> cache-tree.c | 3 +++
>> 1 file changed, 3 insertions(+)
>>
>> diff --git a/cache-tree.c b/cache-tree.c
>> index 45fb57b17f3..f135bb77af5 100644
>> --- a/cache-tree.c
>> +++ b/cache-tree.c
>> @@ -746,7 +746,10 @@ void prime_cache_tree(struct repository *r,
>> {
>> cache_tree_free(&istate->cache_tree);
>> istate->cache_tree = cache_tree();
>> +
>> + trace2_region_enter("cache-tree", "prime_cache_tree", the_repository);
>
> Shouldn't this be at the start of the function, a few lines up?
>
>> prime_cache_tree_rec(r, istate->cache_tree, tree);
>> + trace2_region_leave("cache-tree", "prime_cache_tree", the_repository);
>
> ...and this be one more line down? (or the string "prime_cache_tree"
> have a "_rec" added to it?)
I guess my focus was on creating changes around the "bulk"
of the operation, keeping the region enter/leave pair close
together. But perhaps enclosing the full block will be
better for full coverage in case this method became more
complicated (but not within prime_cache_tree_rec()).
Thanks,
-Stolee
