Re: [PATCH 2/8] unpack-trees: add trace2 regions
- From
Elijah Newren <newren@gmail.com>
- Date
- Dec 30, 2020, 19:45 UTC
- Message-ID
- <CABPp-BGrTcr2q10ppuEML3N1imV_PMm8zzB5SGTQPFWtyUUzEA@mail.gmail.com>
- In-Reply-To
- <6923e6211aaa9e1e144db747c971c6fe35fb4d41.1609356413.git.gitgitgadget@gmail.com>
On Wed, Dec 30, 2020 at 11:26 AM Derrick Stolee via GitGitGadget <gitgitgadget@gmail.com> wrote:
Show 7 quoted lines
> > 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?
Show 41 quoted lines
> 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;
>
> --
> gitgitgadgetSeems simple and straightforward, and I like having more trace2 measurements since I used it so much in merge-ort.