From b6b987a09457e9c8d49aad9170df32b1f607c5b9 Mon Sep 17 00:00:00 2001 From: Elijah Newren Date: Thu, 26 Aug 2010 00:21:46 -0600 Subject: [PATCH 1/4] Document pre-condition for tree_entry_interesting tree_entry_interesting will fail to find appropriate matches if the base directory path is not terminated with a slash. Knowing this earlier would have saved me some debugging time. Signed-off-by: Elijah Newren Signed-off-by: Junio C Hamano --- tree-diff.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tree-diff.c b/tree-diff.c index fe9f52c479..4d85fef5d0 100644 --- a/tree-diff.c +++ b/tree-diff.c @@ -85,6 +85,8 @@ static int compare_tree_entry(struct tree_desc *t1, struct tree_desc *t2, const /* * Is a tree entry interesting given the pathspec we have? * + * Pre-condition: baselen == 0 || base[baselen-1] == '/' + * * Return: * - 2 for "yes, and all subsequent entries will be" * - 1 for yes From dabb061fa358e8884889632f0be5b6c76916c893 Mon Sep 17 00:00:00 2001 From: Elijah Newren Date: Thu, 26 Aug 2010 00:21:47 -0600 Subject: [PATCH 2/4] tree-walk: Correct bitrotted comment about tree_entry() There was a code comment that referred to the "above two functions" but over time the functions immediately preceding the comment have changed. Just mention the relevant functions by name. Signed-off-by: Elijah Newren Signed-off-by: Junio C Hamano --- tree-walk.h | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tree-walk.h b/tree-walk.h index 42110a465f..f78361a676 100644 --- a/tree-walk.h +++ b/tree-walk.h @@ -28,7 +28,10 @@ static inline int tree_entry_len(const char *name, const unsigned char *sha1) void update_tree_entry(struct tree_desc *); void init_tree_desc(struct tree_desc *desc, const void *buf, unsigned long size); -/* Helper function that does both of the above and returns true for success */ +/* + * Helper function that does both tree_entry_extract() and update_tree_entry() + * and returns true for success + */ int tree_entry(struct tree_desc *, struct name_entry *); void *fill_tree_descriptor(struct tree_desc *desc, const unsigned char *sha1); From 4a5e74feb1a75a479acbc2f839e930966fdc2f2f Mon Sep 17 00:00:00 2001 From: Elijah Newren Date: Thu, 26 Aug 2010 00:21:48 -0600 Subject: [PATCH 3/4] tree_entry_interesting(): Make return value more specific tree_entry_interesting() can signal to its callers not only if the given entry matches one of the specified paths, but whether all remaining paths will (or will not) match. When no paths are specified, all paths are considered interesting, so intead of returning 1 (this path is interesting) return 2 (all paths are interesting). This will allow the caller to avoid calling tree_entry_interesting() again, which theoretically should speed up tree walking. I am not able to measure any actual gains in practice, but it certainly can not hurt and seems to make the code more readable to me. Signed-off-by: Elijah Newren Signed-off-by: Junio C Hamano --- tree-diff.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tree-diff.c b/tree-diff.c index 4d85fef5d0..1b41b7f450 100644 --- a/tree-diff.c +++ b/tree-diff.c @@ -103,7 +103,7 @@ static int tree_entry_interesting(struct tree_desc *desc, const char *base, int int never_interesting = -1; if (!opt->nr_paths) - return 1; + return 2; sha1 = tree_entry_extract(desc, &path, &mode); From 7e1ec0d415dbea96d4ec6995c00cb66cdff7ceff Mon Sep 17 00:00:00 2001 From: Elijah Newren Date: Thu, 26 Aug 2010 00:21:49 -0600 Subject: [PATCH 4/4] diff_tree(): Skip skip_uninteresting() when all remaining paths interesting In 1d848f6 (tree_entry_interesting(): allow it to say "everything is interesting" 2007-03-21), both show_tree() and skip_uninteresting() were modified to determine if all remaining tree entries were interesting. However, the latter returns as soon as it finds the first interesting path, without any way to signal to its caller (namely, diff_tree()) that all remaining paths are interesting, making these extra checks useless. Pass whether all remaining entries are interesting back to diff_tree(), and whenever they are, have diff_tree() skip subsequent calls to skip_uninteresting(). With this change, I measure speedups of 3-4% for the commands $ git rev-list --quiet HEAD -- Documentation/ $ git rev-list --quiet HEAD -- t/ in git.git. Signed-off-by: Elijah Newren Signed-off-by: Junio C Hamano --- tree-diff.c | 25 ++++++++++++------------- 1 file changed, 12 insertions(+), 13 deletions(-) diff --git a/tree-diff.c b/tree-diff.c index 1b41b7f450..f6a6b2f71f 100644 --- a/tree-diff.c +++ b/tree-diff.c @@ -259,19 +259,12 @@ static void show_entry(struct diff_options *opt, const char *prefix, struct tree } } -static void skip_uninteresting(struct tree_desc *t, const char *base, int baselen, struct diff_options *opt) +static void skip_uninteresting(struct tree_desc *t, const char *base, int baselen, struct diff_options *opt, int *all_interesting) { - int all_interesting = 0; while (t->size) { - int show; - - if (all_interesting) - show = 1; - else { - show = tree_entry_interesting(t, base, baselen, opt); - if (show == 2) - all_interesting = 1; - } + int show = tree_entry_interesting(t, base, baselen, opt); + if (show == 2) + *all_interesting = 1; if (!show) { update_tree_entry(t); continue; @@ -286,14 +279,20 @@ static void skip_uninteresting(struct tree_desc *t, const char *base, int basele int diff_tree(struct tree_desc *t1, struct tree_desc *t2, const char *base, struct diff_options *opt) { int baselen = strlen(base); + int all_t1_interesting = 0; + int all_t2_interesting = 0; for (;;) { if (DIFF_OPT_TST(opt, QUICK) && DIFF_OPT_TST(opt, HAS_CHANGES)) break; if (opt->nr_paths) { - skip_uninteresting(t1, base, baselen, opt); - skip_uninteresting(t2, base, baselen, opt); + if (!all_t1_interesting) + skip_uninteresting(t1, base, baselen, opt, + &all_t1_interesting); + if (!all_t2_interesting) + skip_uninteresting(t2, base, baselen, opt, + &all_t2_interesting); } if (!t1->size) { if (!t2->size)