diff mbox series

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

Message ID 6923e6211aaa9e1e144db747c971c6fe35fb4d41.1609356413.git.gitgitgadget@gmail.com (mailing list archive)
State New, archived
Headers show
Series Cleanups around index operations | expand

Commit Message

Derrick Stolee Dec. 30, 2020, 7:26 p.m. UTC
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.

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(+)

Comments

Elijah Newren Dec. 30, 2020, 7:45 p.m. UTC | #1
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.
diff mbox series

Patch

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;