Re: [PATCH v3 1/1] fetch: add trace2 instrumentation
"Erik Chen via GitGitGadget" <gitgitgadget@gmail.com> writes:
Show 14 quoted lines
> From: Erik Chen <erikchen@chromium.org>
>
> Add trace2 regions to fetch-pack.c to better track time spent in the various
> phases of a fetch:
>
> * matching common remote and local refs
> * marking local refs as complete (part of the matching process)
>
> Both of these stages can be slow for repositories with many refs.
>
> Signed-off-by: Erik Chen <erikchen@chromium.org>
> ---
> fetch-pack.c | 8 ++++++++
> 1 file changed, 8 insertions(+)
Show 20 quoted lines
> diff --git a/fetch-pack.c b/fetch-pack.c
> index 0130b44112..5e3eee0477 100644
> --- a/fetch-pack.c
> +++ b/fetch-pack.c
> @@ -669,6 +669,8 @@ static void mark_complete_and_common_ref(struct fetch_negotiator *negotiator,
>
> save_commit_buffer = 0;
>
> + trace2_region_enter("fetch-pack", "mark_complete_and_common_ref", NULL);
> +
> for (ref = *refs; ref; ref = ref->next) {
> struct object *o;
>
> @@ -690,6 +692,10 @@ static void mark_complete_and_common_ref(struct fetch_negotiator *negotiator,
> }
> }
>
> + /* This block marks all local refs as COMPLETE, and then recursively marks all
> + * parents of those refs as COMPLETE.
> + */ /*
* 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).
Show 17 quoted lines
> + trace2_region_enter("fetch-pack", "mark_complete_local_refs", NULL);
> if (!args->deepen) {
> for_each_ref(mark_complete_oid, NULL);
> for_each_cached_alternate(NULL, mark_alternate_complete);
> @@ -697,6 +703,7 @@ static void mark_complete_and_common_ref(struct fetch_negotiator *negotiator,
> if (cutoff)
> mark_recent_complete_commits(args, cutoff);
> }
> + trace2_region_leave("fetch-pack", "mark_complete_local_refs", NULL);
>
> /*
> * Mark all complete remote refs as common refs.
> @@ -716,6 +723,7 @@ static void mark_complete_and_common_ref(struct fetch_negotiator *negotiator,
> }
>
> save_commit_buffer = old_save_commit_buffer;
> + trace2_region_leave("fetch-pack", "mark_complete_and_common_ref", NULL);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 () {
<region 1 begin>
<region 1.1 begin>
code
<region 1.1 end>
<region 1.2 begin>
code
<region 1.2 end>
<region 1.3 begin>
code
<region 1.3 end>
<region 1 end>
}or
F () {
trivial code
<region 1 begin>
heavy code
<region 1 end>
trivial code
}but this appears to do
F () {
<region 1 begin>
code
<region 1.1 begin>
code
<region 1.1 end>
code
<region 1 end>
}which is somewhat puzzling.
Thanks.
