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

From: Erik Chen <erikchen@chromium.org>
Date: Nov 19, 2019, 21:47 UTC
Message-ID: <383762c9-06eb-707c-c0d8-c8ec8757133e@chromium.org>
In-Reply-To: <xmqq5zjws12y.fsf@gitster-ct.c.googlers.com>

Importing response from https://github.com/gitgitgadget/git/pull/451#issuecomment-555044068

Show 10 quoted lines

> 
>          /*
>           * We write our multi-line comments like this, with the
>           * slash-asterisk at the beginning and the asterisk-slash
>           * at the end on its own line.  Learn such local conventions
>           * from the existing surrounding code and imitate, which
>           * would reduce stylistic errors.
>           */
> 
> Will fix-up while queuing (no need to reroll only to fix this).

Sorry about that. I was copying from line 682, which looks like it was also using the wrong style. I've fixed that line as well.

Show 5 quoted lines

> 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

Good point. I used this structure because I observed locally that the middle block was slow, but was concerned that the initial/ending block might be slow as well. I've gone ahead and adding tracing regions for each block.