Re: [PATCH v3 1/1] fetch: add trace2 instrumentation

[Junio C Hamano <gitster@pobox.com>]

Derrick Stolee <stolee@gmail.com> writes:

> On 11/7/2019 12:32 AM, Junio C Hamano wrote:
>> So this introduces a single region around the entire function body
>> of mark_complete_and_common_ref(), within which only one subpart is
>> also enclosed in a nested region.  Is that because the parts inside
>> the outer region before and after the inner region are known to
>> consume negligible time?  IOW I would understand
>> 
>>         F () {
>> ...
>>         }
>> 
>> or
>> 
>>         F () {
>>                         trivial code
>>                 <region 1 begin>
>>                         heavy code
>>                 <region 1 end>
>>                         trivial code
>>         }
>> 
>> but this appears to do
>> ...
>> which is somewhat puzzling.
>
> I notice that a v4 was sent that adds more sub-regions without actually
> responding to this request. (It is worth also pointing out that you
> ignored Junio's request you use the cover letter to explain your reasoning
> for changes between versions.)

Thanks for noticing.  I wasn't requesting any change in particular
(at least not yet), but was inquiring the reasoning behind what was
done.  From that point of view, the lack of answers was worse than
yet another patch that does not explain why it was done that other
way this time around.

> There is a real downside to nesting regions like this. Specifically, we
> frequently limit the depth that we report nested regions to avoid
> overwhelming the logs.
>
> In general, these sub-regions should be avoided when possible and instead
> create regions around important sections, such as the second option Junio
> lists above.

Thanks for a clear direction as the area expert of trace2.