Re: [PATCH 5/8] cache-tree: trace regions for prime_cache_tree
- From
Derrick Stolee <stolee@gmail.com>
- Date
- Dec 30, 2020, 19:53 UTC
- Message-ID
- <97488332-a24e-889c-0114-ff54b12e573b@gmail.com>
- In-Reply-To
- <CABPp-BGTXvNjwQkqSzqLJwL6T2a_Ly_9=wiejqpt-3sq6oPGEw@mail.gmail.com>
On 12/30/2020 2:48 PM, Elijah Newren wrote:
Show 35 quoted lines
> On Wed, Dec 30, 2020 at 11:26 AM Derrick Stolee via GitGitGadget
> <gitgitgadget@gmail.com> wrote:
>>
>> From: Derrick Stolee <dstolee@microsoft.com>
>>
>> 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@microsoft.com>
>> ---
>> 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