From: Erik Chen <erikchen@chromium.org>
Date: Tue, 19 Nov 2019 21:24:50 GMT
Subject: Re: [PATCH v3 1/1] fetch: add trace2 instrumentation
Message-ID: <8b6d4bdf-d982-97ad-e475-672e6ea914c2@chromium.org>
In-Reply-To: <xmqqsgmkzl1s.fsf@gitster-ct.c.googlers.com>


On 11/18/19 5:55 PM, Junio C Hamano wrote:
> 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.

Sorry, I've been replying on the GitGitGadget pull request thread:

e.g. https://github.com/gitgitgadget/git/pull/451#issuecomment-555044068


But none of those replies have been making their way to this mailing 
list. I'll attempt to send these replies to the mailing list now.


>
>> 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.
>
>