Re: [PATCH 2/8] unpack-trees: add trace2 regions

[Elijah Newren <newren@gmail.com>]

On Wed, Dec 30, 2020 at 11:26 AM Derrick Stolee via GitGitGadget
<gitgitgadget@gmail.com> wrote:
>
> From: Derrick Stolee <dstolee@microsoft.com>
>
> The unpack_trees() method is quite complicated and its performance can
> change dramatically depending on how it is used. We already have some
> performance tracing regions, but they have not been updated to the
> trace2 API. Do so now.

Somewhat of a curious side comment: Any idea at what scale the perf
issues show up?  Or are you still digging into that?

> We already have trace2 regions in unpack_trees.c:clear_ce_flags(), which
> uses a linear scan through the index without recursing into trees.
>
> Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
> ---
>  unpack-trees.c | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/unpack-trees.c b/unpack-trees.c
> index 02f484604ac..7dbd006ac56 100644
> --- a/unpack-trees.c
> +++ b/unpack-trees.c
> @@ -1579,6 +1579,8 @@ int unpack_trees(unsigned len, struct tree_desc *t, struct unpack_trees_options
>                 die("unpack_trees takes at most %d trees", MAX_UNPACK_TREES);
>
>         trace_performance_enter();
> +       trace2_region_enter("unpack_trees", "unpack_trees", the_repository);
> +
>         if (!core_apply_sparse_checkout || !o->update)
>                 o->skip_sparse_checkout = 1;
>         if (!o->skip_sparse_checkout && !o->pl) {
> @@ -1652,7 +1654,9 @@ int unpack_trees(unsigned len, struct tree_desc *t, struct unpack_trees_options
>                 }
>
>                 trace_performance_enter();
> +               trace2_region_enter("unpack_trees", "traverse_trees", the_repository);
>                 ret = traverse_trees(o->src_index, len, t, &info);
> +               trace2_region_leave("unpack_trees", "traverse_trees", the_repository);
>                 trace_performance_leave("traverse_trees");
>                 if (ret < 0)
>                         goto return_failed;
> @@ -1740,6 +1744,7 @@ int unpack_trees(unsigned len, struct tree_desc *t, struct unpack_trees_options
>  done:
>         if (free_pattern_list)
>                 clear_pattern_list(&pl);
> +       trace2_region_leave("unpack_trees", "unpack_trees", the_repository);
>         trace_performance_leave("unpack_trees");
>         return ret;
>
> --
> gitgitgadget

Seems simple and straightforward, and I like having more trace2
measurements since I used it so much in merge-ort.