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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Derrick Stolee <stolee@xxxxxxxxx> 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.



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux