Re: [PATCH 0/2] add trace2 regions to fetch & push
Josh Steadmon <steadmon@google.com> writes:
Show 19 quoted lines
> On 2019.10.07 14:46, Jonathan Tan wrote:
>> > We'd like to collect better statistics about where the time is spent in
>> > fetches and pushes so that we can hopefully identify some areas for
>> > future optimization. So let's add some trace2 regions around some of the
>> > fetch/push phases so we can break down their timing.
>>
>> Thanks.
>>
>> Patch 1 looks good to me - different regions at the same level
>> (builtin/fetch.c, so it will be just for "git fetch") and one specific
>> one just for negotiation, which has to be in fetch-pack.c because only
>> that file operates at that level.
>>
>> Patch 2 mostly looks good to me too - unlike fetch, a lot happens in
>> transport.c, so it's reasonable to put most of the regions there. One
>> comment: in transport_push(), should trace2_region_{enter,leave} take
>> "r" instead of "the_repository"?
>
> Ah yeah, thanks for the catch. Fixed in V2.Yuck. It's already in 'next', isn't it?