From fef9b282031be27b87a70a1fcb80b0cfd6f56e2c Mon Sep 17 00:00:00 2001 From: Derrick Stolee Date: Mon, 29 Apr 2019 12:49:45 -0400 Subject: [PATCH] unpack-trees: add trace2 regions around each use The unpack_trees() method has many uses, and each can have different performance characteristics. Create trace2 regions around each call so we can measure the cost of this method in each situation. These instances were found by grepping for the setup_unpack_trees_porcelain() method, which is called before every actual call to unpack_trees(). Searching for unpack_trees() finds many false-positives in comments and documentation. Signed-off-by: Derrick Stolee --- builtin/checkout.c | 3 +++ builtin/rebase.c | 8 ++++++-- merge-recursive.c | 3 +++ merge.c | 8 ++++++-- sequencer.c | 8 ++++++-- 5 files changed, 24 insertions(+), 6 deletions(-) diff --git a/builtin/checkout.c b/builtin/checkout.c index 9d8a69331adbd7..1f3b738e2c812d 100644 --- a/builtin/checkout.c +++ b/builtin/checkout.c @@ -632,7 +632,10 @@ static int merge_working_tree(const struct checkout_opts *opts, tree = parse_tree_indirect(&new_branch_info->commit->object.oid); init_tree_desc(&trees[1], tree->buffer, tree->size); + trace2_region_enter("unpack-trees", "checkout", the_repository); ret = unpack_trees(2, trees, &topts); + trace2_region_leave("unpack-trees", "checkout", the_repository); + clear_unpack_trees_porcelain(&topts); if (ret == -1) { /* diff --git a/builtin/rebase.c b/builtin/rebase.c index 2fc253e7b21922..adfd0cf0efb943 100644 --- a/builtin/rebase.c +++ b/builtin/rebase.c @@ -387,7 +387,7 @@ static int reset_head(struct object_id *oid, const char *action, size_t prefix_len; struct object_id *orig = NULL, oid_orig, *old_orig = NULL, oid_old_orig; - int ret = 0, nr = 0; + int ret = 0, nr = 0, unpack_trees_result; if (switch_to_branch && !starts_with(switch_to_branch, "refs/")) BUG("Not a fully qualified branch: '%s'", switch_to_branch); @@ -435,7 +435,11 @@ static int reset_head(struct object_id *oid, const char *action, goto leave_reset_head; } - if (unpack_trees(nr, desc, &unpack_tree_opts)) { + trace2_region_enter("unpack-trees", "rebase", the_repository); + unpack_trees_result = unpack_trees(nr, desc, &unpack_tree_opts); + trace2_region_leave("unpack-trees", "rebase", the_repository); + + if (unpack_trees_result) { ret = -1; goto leave_reset_head; } diff --git a/merge-recursive.c b/merge-recursive.c index 5938f4517ed72b..2495eb40ff03b4 100644 --- a/merge-recursive.c +++ b/merge-recursive.c @@ -389,7 +389,10 @@ static int unpack_trees_start(struct merge_options *o, init_tree_desc_from_tree(t+1, head); init_tree_desc_from_tree(t+2, merge); + trace2_region_enter("unpack-trees", "merge-recursive", the_repository); rc = unpack_trees(3, t, &o->unpack_opts); + trace2_region_leave("unpack-trees", "merge-recursive", the_repository); + cache_tree_free(&active_cache_tree); /* diff --git a/merge.c b/merge.c index 91008f760223e4..a022adc273c0f5 100644 --- a/merge.c +++ b/merge.c @@ -52,7 +52,7 @@ int checkout_fast_forward(struct repository *r, struct tree *trees[MAX_UNPACK_TREES]; struct unpack_trees_options opts; struct tree_desc t[MAX_UNPACK_TREES]; - int i, nr_trees = 0; + int i, nr_trees = 0, unpack_trees_result; struct dir_struct dir; struct lock_file lock_file = LOCK_INIT; @@ -96,7 +96,11 @@ int checkout_fast_forward(struct repository *r, opts.fn = twoway_merge; setup_unpack_trees_porcelain(&opts, "merge"); - if (unpack_trees(nr_trees, t, &opts)) { + trace2_region_enter("unpack-trees", "merge", r); + unpack_trees_result = unpack_trees(nr_trees, t, &opts); + trace2_region_leave("unpack-trees", "merge", r); + + if (unpack_trees_result) { rollback_lock_file(&lock_file); clear_unpack_trees_porcelain(&opts); return -1; diff --git a/sequencer.c b/sequencer.c index 4eaa265cc4161c..d9f87ea2886508 100644 --- a/sequencer.c +++ b/sequencer.c @@ -2941,7 +2941,7 @@ static int do_reset(const char *name, int len, struct replay_opts *opts) struct tree_desc desc; struct tree *tree; struct unpack_trees_options unpack_tree_opts; - int ret = 0; + int ret = 0, unpack_trees_result; if (hold_locked_index(&lock, LOCK_REPORT_ON_ERROR) < 0) return -1; @@ -3002,7 +3002,11 @@ static int do_reset(const char *name, int len, struct replay_opts *opts) return -1; } - if (unpack_trees(1, &desc, &unpack_tree_opts)) { + trace2_region_enter("unpack-trees", "sequencer", the_repository); + unpack_trees_result = unpack_trees(1, &desc, &unpack_tree_opts); + trace2_region_leave("unpack-trees", "sequencer", the_repository); + + if (unpack_trees_result) { rollback_lock_file(&lock); free((void *)desc.buffer); strbuf_release(&ref_name);