[PATCH 1/2] diff --quick

[PATCH 1/2] diff --quick

[Junio C Hamano]

This adds the command line option 'quick' to tell 'git diff-*' that
we are not interested in the actual diff contents but only if there
is any change.  This option automatically turns --exit-code on, and
turns off output formatting, as it does not make much sense to show
the first hit we happened to have found.

The backends have not been taught about the option with this patch.
That is a topic for later rounds.

Signed-off-by: Junio C Hamano <junkio@cox.net>
---

 * Obviously this comes on top of Alex's --exit-code.

 diff.c |   28 ++++++++++++++++++++++++++--
 diff.h |    4 ++--
 2 files changed, 28 insertions(+), 4 deletions(-)

diff --git a/diff.c b/diff.c
index 7d938c1..99f5802 100644
--- a/diff.c
+++ b/diff.c
@@ -1958,6 +1958,23 @@ int diff_setup_done(struct diff_options *options)
 	if (options->abbrev <= 0 || 40 < options->abbrev)
 		options->abbrev = 40; /* full */
 
+	/*
+	 * It does not make sense to show the first hit we happened
+	 * to have found.  It does not make sense not to return with
+	 * exit code in such a case either.
+	 */
+	if (options->quick) {
+		options->output_format = DIFF_FORMAT_NO_OUTPUT;
+		options->exit_with_status = 1;
+	}
+
+	/*
+	 * If we postprocess in diffcore, we cannot simply return
+	 * upon the first hit.  We need to run diff as usual.
+	 */
+	if (options->pickaxe || options->filter)
+		options->quick = 0;
+
 	return 0;
 }
 
@@ -2136,6 +2153,8 @@ int diff_opt_parse(struct diff_options *options, const char **av, int ac)
 		options->detect_rename = 0;
 	else if (!strcmp(arg, "--exit-code"))
 		options->exit_with_status = 1;
+	else if (!strcmp(arg, "--quick"))
+		options->quick = 1;
 	else
 		return 0;
 	return 1;
@@ -2900,6 +2919,11 @@ static void diffcore_apply_filter(const char *filter)
 
 void diffcore_std(struct diff_options *options)
 {
+	if (options->quick) {
+		options->has_changes = !!diff_queued_diff.nr;
+		return;
+	}
+
 	if (options->break_opt != -1)
 		diffcore_break(options->break_opt);
 	if (options->detect_rename)
@@ -2912,8 +2936,8 @@ void diffcore_std(struct diff_options *options)
 		diffcore_order(options->orderfile);
 	diff_resolve_rename_copy();
 	diffcore_apply_filter(options->filter);
-	if (options->exit_with_status)
-		options->has_changes = !!diff_queued_diff.nr;
+
+	options->has_changes = !!diff_queued_diff.nr;
 }
 
 
diff --git a/diff.h b/diff.h
index 81fa265..2555120 100644
--- a/diff.h
+++ b/diff.h
@@ -57,6 +57,8 @@ struct diff_options {
 		 find_copies_harder:1,
 		 color_diff:1,
 		 color_diff_words:1,
+		 has_changes:1,
+		 quick:1,
 		 exit_with_status:1;
 	int context;
 	int break_opt;
@@ -72,8 +74,6 @@ struct diff_options {
 	const char *msg_sep;
 	const char *stat_sep;
 	long xdl_opts;
-	/* 0 - no differences; only meaningful if exit_with_status set */
-	int has_changes;
 
 	int stat_width;
 	int stat_name_width;
-- 
1.5.0.3.1036.g6baf1

[PATCH 1/5] Remove unused diffcore_std_no_resolve

[Junio C Hamano]

This was only used by diff-tree-helper program, whose purpose
was to translate a raw diff to a patch.

Signed-off-by: Junio C Hamano <junkio@cox.net>
---
 * These are re-spin of the earlier 2 patch series, go directly
   on top of Alex's --exit-code.

 diff.c |   11 -----------
 diff.h |    2 --
 2 files changed, 0 insertions(+), 13 deletions(-)

diff --git a/diff.c b/diff.c
index cc81801..7d938c1 100644
--- a/diff.c
+++ b/diff.c
@@ -2917,17 +2917,6 @@ void diffcore_std(struct diff_options *options)
 }
 
 
-void diffcore_std_no_resolve(struct diff_options *options)
-{
-	if (options->pickaxe)
-		diffcore_pickaxe(options->pickaxe, options->pickaxe_opts);
-	if (options->orderfile)
-		diffcore_order(options->orderfile);
-	diffcore_apply_filter(options->filter);
-	if (options->exit_with_status)
-		options->has_changes = !!diff_queued_diff.nr;
-}
-
 void diff_addremove(struct diff_options *options,
 		    int addremove, unsigned mode,
 		    const unsigned char *sha1,
diff --git a/diff.h b/diff.h
index d13fc89..81fa265 100644
--- a/diff.h
+++ b/diff.h
@@ -173,8 +173,6 @@ extern int diff_setup_done(struct diff_options *);
 
 extern void diffcore_std(struct diff_options *);
 
-extern void diffcore_std_no_resolve(struct diff_options *);
-
 #define COMMON_DIFF_OPTIONS_HELP \
 "\ncommon diff options:\n" \
 "  -z            output diff-raw with lines terminated with NUL.\n" \
-- 
1.5.0.3.1036.g6baf1

[PATCH 2/5] diff --quiet

[Junio C Hamano]

This adds the command line option 'quiet' to tell 'git diff-*'
that we are not interested in the actual diff contents but only
want to know if there is any change.  This option automatically
turns --exit-code on, and turns off output formatting, as it
does not make much sense to show the first hit we happened to
have found.

The --quiet option is silently turned off (but --exit-code is
still in effect, so is silent output) if postprocessing filters
such as pickaxe and diff-filter are used.  For all practical
purposes I do not think of a reason to want to use these filters
and not viewing the diff output.

The backends have not been taught about the option with this patch.
That is a topic for later rounds.

Signed-off-by: Junio C Hamano <junkio@cox.net>
---
 diff.c |   27 +++++++++++++++++++++++++--
 diff.h |    4 ++--
 2 files changed, 27 insertions(+), 4 deletions(-)

diff --git a/diff.c b/diff.c
index 7d938c1..d8f9242 100644
--- a/diff.c
+++ b/diff.c
@@ -1958,6 +1958,23 @@ int diff_setup_done(struct diff_options *options)
 	if (options->abbrev <= 0 || 40 < options->abbrev)
 		options->abbrev = 40; /* full */
 
+	/*
+	 * It does not make sense to show the first hit we happened
+	 * to have found.  It does not make sense not to return with
+	 * exit code in such a case either.
+	 */
+	if (options->quiet) {
+		options->output_format = DIFF_FORMAT_NO_OUTPUT;
+		options->exit_with_status = 1;
+	}
+
+	/*
+	 * If we postprocess in diffcore, we cannot simply return
+	 * upon the first hit.  We need to run diff as usual.
+	 */
+	if (options->pickaxe || options->filter)
+		options->quiet = 0;
+
 	return 0;
 }
 
@@ -2136,6 +2153,8 @@ int diff_opt_parse(struct diff_options *options, const char **av, int ac)
 		options->detect_rename = 0;
 	else if (!strcmp(arg, "--exit-code"))
 		options->exit_with_status = 1;
+	else if (!strcmp(arg, "--quiet"))
+		options->quiet = 1;
 	else
 		return 0;
 	return 1;
@@ -2900,6 +2919,8 @@ static void diffcore_apply_filter(const char *filter)
 
 void diffcore_std(struct diff_options *options)
 {
+	if (options->quiet)
+		return;
 	if (options->break_opt != -1)
 		diffcore_break(options->break_opt);
 	if (options->detect_rename)
@@ -2912,8 +2933,8 @@ void diffcore_std(struct diff_options *options)
 		diffcore_order(options->orderfile);
 	diff_resolve_rename_copy();
 	diffcore_apply_filter(options->filter);
-	if (options->exit_with_status)
-		options->has_changes = !!diff_queued_diff.nr;
+
+	options->has_changes = !!diff_queued_diff.nr;
 }
 
 
@@ -2952,6 +2973,7 @@ void diff_addremove(struct diff_options *options,
 		fill_filespec(two, sha1, mode);
 
 	diff_queue(&diff_queued_diff, one, two);
+	options->has_changes = 1;
 }
 
 void diff_change(struct diff_options *options,
@@ -2977,6 +2999,7 @@ void diff_change(struct diff_options *options,
 	fill_filespec(two, new_sha1, new_mode);
 
 	diff_queue(&diff_queued_diff, one, two);
+	options->has_changes = 1;
 }
 
 void diff_unmerge(struct diff_options *options,
diff --git a/diff.h b/diff.h
index 81fa265..a0d2ce1 100644
--- a/diff.h
+++ b/diff.h
@@ -57,6 +57,8 @@ struct diff_options {
 		 find_copies_harder:1,
 		 color_diff:1,
 		 color_diff_words:1,
+		 has_changes:1,
+		 quiet:1,
 		 exit_with_status:1;
 	int context;
 	int break_opt;
@@ -72,8 +74,6 @@ struct diff_options {
 	const char *msg_sep;
 	const char *stat_sep;
 	long xdl_opts;
-	/* 0 - no differences; only meaningful if exit_with_status set */
-	int has_changes;
 
 	int stat_width;
 	int stat_name_width;
-- 
1.5.0.3.1036.g6baf1

[PATCH 3/5] Teach --quiet to diff backends.

[Junio C Hamano]

This teaches git-diff-files, git-diff-index and git-diff-tree
backends to exit early under --quiet option.

Signed-off-by: Junio C Hamano <junkio@cox.net>
---
 diff-lib.c  |    6 ++++++
 tree-diff.c |    2 ++
 2 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/diff-lib.c b/diff-lib.c
index f9a1a10..5c5b05b 100644
--- a/diff-lib.c
+++ b/diff-lib.c
@@ -324,6 +324,9 @@ int run_diff_files(struct rev_info *revs, int silent_on_removed)
 		struct cache_entry *ce = active_cache[i];
 		int changed;
 
+		if (revs->diffopt.quiet && revs->diffopt.has_changes)
+			break;
+
 		if (!ce_path_match(ce, revs->prune_data))
 			continue;
 
@@ -565,6 +568,9 @@ static int diff_cache(struct rev_info *revs,
 		struct cache_entry *ce = *ac;
 		int same = (entries > 1) && ce_same_name(ce, ac[1]);
 
+		if (revs->diffopt.quiet && revs->diffopt.has_changes)
+			break;
+
 		if (!ce_path_match(ce, pathspec))
 			goto skip_entry;
 
diff --git a/tree-diff.c b/tree-diff.c
index c827582..44cde74 100644
--- a/tree-diff.c
+++ b/tree-diff.c
@@ -161,6 +161,8 @@ static void show_entry(struct diff_options *opt, const char *prefix, struct tree
 int diff_tree(struct tree_desc *t1, struct tree_desc *t2, const char *base, struct diff_options *opt)
 {
 	while (t1->size | t2->size) {
+		if (opt->quiet && opt->has_changes)
+			break;
 		if (opt->nr_paths && t1->size && !interesting(t1, base, opt)) {
 			update_tree_entry(t1);
 			continue;
-- 
1.5.0.3.1036.g6baf1

[PATCH 4/5] revision.c: explain what tree_difference does

[Junio C Hamano]

This explains how tree_difference variable is used, and updates two
places where the code knows symbolic constant REV_TREE_SAME is 0.

Signed-off-by: Junio C Hamano <junkio@cox.net>
---
 revision.c |   11 +++++++++--
 1 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/revision.c b/revision.c
index 3c2eb12..129d197 100644
--- a/revision.c
+++ b/revision.c
@@ -213,6 +213,13 @@ static int everybody_uninteresting(struct commit_list *orig)
 	return 1;
 }
 
+/*
+ * The goal is to get REV_TREE_NEW as the result only if the
+ * diff consists of all '+' (and no other changes), and
+ * REV_TREE_DIFFERENT otherwise (of course if the trees are
+ * the same we want REV_TREE_SAME).  That means that once we
+ * get to REV_TREE_DIFFERENT, we do not have to look any further.
+ */
 static int tree_difference = REV_TREE_SAME;
 
 static void file_add_remove(struct diff_options *options,
@@ -277,11 +284,11 @@ int rev_same_tree_as_empty(struct rev_info *revs, struct tree *t1)
 	empty.buf = "";
 	empty.size = 0;
 
-	tree_difference = 0;
+	tree_difference = REV_TREE_SAME;
 	retval = diff_tree(&empty, &real, "", &revs->pruning);
 	free(tree);
 
-	return retval >= 0 && !tree_difference;
+	return retval >= 0 && (tree_difference == REV_TREE_SAME);
 }
 
 static void try_to_simplify_commit(struct rev_info *revs, struct commit *commit)
-- 
1.5.0.3.1036.g6baf1

[PATCH 5/5] try-to-simplify-commit: use diff-tree --quiet machinery.

[Junio C Hamano]

This uses diff-tree --quiet machinery to terminate the internal
diff-tree between a commit and its parents via revs.pruning (not
revs.diffopt) as soon as we find enough about the tree change.

With respect to the optionally given pathspec, we are interested
if the tree of commit is identical to the parent's, only adds
new paths to the parent's, or there are other differences.  As
soon as we find out that there is one such other kind of
difference, we do not have to compare the rest of the tree.

Because we do not call standard diff_addremove/diff_change, we
instruct the diff-tree machinery to stop early by setting
has_changes when we say we found the trees to be different.

Signed-off-by: Junio C Hamano <junkio@cox.net>
---
 revision.c |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/revision.c b/revision.c
index 129d197..bcdb6a1 100644
--- a/revision.c
+++ b/revision.c
@@ -243,6 +243,8 @@ static void file_add_remove(struct diff_options *options,
 		diff = REV_TREE_NEW;
 	}
 	tree_difference = diff;
+	if (tree_difference == REV_TREE_DIFFERENT)
+		options->has_changes = 1;
 }
 
 static void file_change(struct diff_options *options,
@@ -252,6 +254,7 @@ static void file_change(struct diff_options *options,
 		 const char *base, const char *path)
 {
 	tree_difference = REV_TREE_DIFFERENT;
+	options->has_changes = 1;
 }
 
 int rev_compare_tree(struct rev_info *revs, struct tree *t1, struct tree *t2)
@@ -261,6 +264,7 @@ int rev_compare_tree(struct rev_info *revs, struct tree *t1, struct tree *t2)
 	if (!t2)
 		return REV_TREE_DIFFERENT;
 	tree_difference = REV_TREE_SAME;
+	revs->pruning.has_changes = 0;
 	if (diff_tree_sha1(t1->object.sha1, t2->object.sha1, "",
 			   &revs->pruning) < 0)
 		return REV_TREE_DIFFERENT;
@@ -285,6 +289,7 @@ int rev_same_tree_as_empty(struct rev_info *revs, struct tree *t1)
 	empty.size = 0;
 
 	tree_difference = REV_TREE_SAME;
+	revs->pruning.has_changes = 0;
 	retval = diff_tree(&empty, &real, "", &revs->pruning);
 	free(tree);
 
@@ -552,6 +557,7 @@ void init_revisions(struct rev_info *revs, const char *prefix)
 	revs->ignore_merges = 1;
 	revs->simplify_history = 1;
 	revs->pruning.recursive = 1;
+	revs->pruning.quiet = 1;
 	revs->pruning.add_remove = file_add_remove;
 	revs->pruning.change = file_change;
 	revs->lifo = 1;
-- 
1.5.0.3.1036.g6baf1

Re: [PATCH 2/5] diff --quiet

[Alex Riesen]

On 3/14/07, Junio C Hamano <junkio@cox.net> wrote:
> The --quiet option is silently turned off (but --exit-code is
> still in effect, so is silent output) if postprocessing filters
> such as pickaxe and diff-filter are used.  For all practical
> purposes I do not think of a reason to want to use these filters
> and not viewing the diff output.

pickaxe filter can be useful in hooks to enforce policy
on use of some constructs. I can well imagine (do not
even have to imagine) a stupid company policy forbidding
commits with "\<printf\>" in them. Have no idea for
diff-filter though.

Re: [PATCH 2/5] diff --quiet

[Alex Riesen]

On 3/14/07, Alex Riesen <raa.lkml@gmail.com> wrote:
> On 3/14/07, Junio C Hamano <junkio@cox.net> wrote:
> > The --quiet option is silently turned off (but --exit-code is
> > still in effect, so is silent output) if postprocessing filters
> > such as pickaxe and diff-filter are used.  For all practical
> > purposes I do not think of a reason to want to use these filters
> > and not viewing the diff output.
>
> pickaxe filter can be useful in hooks to enforce policy

Actually, I find it a bit strange to leave the output silent,
if --quiet is turned off. For all the user sees, it is still quiet
and exit-coded, just slow. Suggest either restore the
output format when turning the option off or not turning
it off at all (if possible. Is it?)

Re: [PATCH 2/5] diff --quiet

[Alex Riesen]

On 3/14/07, Junio C Hamano <junkio@cox.net> wrote:
> This adds the command line option 'quiet' to tell 'git diff-*'
> that we are not interested in the actual diff contents but only
> want to know if there is any change.  This option automatically
> turns --exit-code on, and turns off output formatting, as it
> does not make much sense to show the first hit we happened to
> have found.

Now I'm happy :)

~/linux$ time git diff-tree -r -s v2.6.16 v2.6.20

real    0m0.137s
user    0m0.117s
sys     0m0.020s
~/linux$ time ~/projects/git-diff/git-diff-tree -r --quiet v2.6.16 v2.6.20

real    0m0.006s
user    0m0.000s
sys     0m0.007s

Re: [PATCH 2/5] diff --quiet

[Junio C Hamano]

"Alex Riesen" <raa.lkml@gmail.com> writes:

> Actually, I find it a bit strange to leave the output silent,
> if --quiet is turned off. For all the user sees, it is still quiet
> and exit-coded

For that exact reason, for all the user cares, it is better to
leave the output turned off and result given back with exit
status.  --quiet is really for scripted use, in other words.

I wonder if we should perhaps turn off the PAGER when --exit-code
or --quiet is given.

Re: [PATCH 2/5] diff --quiet

[Junio C Hamano]

"Alex Riesen" <raa.lkml@gmail.com> writes:

> Now I'm happy :)
>
> ~/linux$ time git diff-tree -r -s v2.6.16 v2.6.20
>
> real    0m0.137s
> user    0m0.117s
> sys     0m0.020s
> ~/linux$ time ~/projects/git-diff/git-diff-tree -r --quiet v2.6.16 v2.6.20
>
> real    0m0.006s
> user    0m0.000s
> sys     0m0.007s

You do not need diff-tree --quiet to do that!

	$ git-rev-parse v2.6.16^{tree} v2.6.20^{tree}

Re: [PATCH 2/5] diff --quiet

[Linus Torvalds]

On Wed, 14 Mar 2007, Junio C Hamano wrote:

> "Alex Riesen" <raa.lkml@gmail.com> writes:
> 
> > Now I'm happy :)
> >
> > ~/linux$ time git diff-tree -r -s v2.6.16 v2.6.20
> >
> > real    0m0.137s
> > user    0m0.117s
> > sys     0m0.020s
> > ~/linux$ time ~/projects/git-diff/git-diff-tree -r --quiet v2.6.16 v2.6.20
> >
> > real    0m0.006s
> > user    0m0.000s
> > sys     0m0.007s
> 
> You do not need diff-tree --quiet to do that!
> 
> 	$ git-rev-parse v2.6.16^{tree} v2.6.20^{tree}

Well, if you have a path-spec, it can certainly matter.

Personally, I think it's more interesting if this can make a difference 
for something like

	git log v2.6.12.. -- drivers/ > /dev/null

but that would require that we actually understand that we can stop early 
if we ever get to REV_TREE_DIFFERENT. I didn't check if you actually did 
that optimization.

		Linus

Re: [PATCH 2/5] diff --quiet

[Junio C Hamano]

Linus Torvalds <torvalds@linux-foundation.org> writes:

> On Wed, 14 Mar 2007, Junio C Hamano wrote:
>
>> "Alex Riesen" <raa.lkml@gmail.com> writes:
>> 
>> > Now I'm happy :)
>> >
>> > ~/linux$ time git diff-tree -r -s v2.6.16 v2.6.20
>> >
>> > real    0m0.137s
>> > user    0m0.117s
>> > sys     0m0.020s
>> > ~/linux$ time ~/projects/git-diff/git-diff-tree -r --quiet v2.6.16 v2.6.20
>> >
>> > real    0m0.006s
>> > user    0m0.000s
>> > sys     0m0.007s
>> 
>> You do not need diff-tree --quiet to do that!
>> 
>> 	$ git-rev-parse v2.6.16^{tree} v2.6.20^{tree}
>
> Well, if you have a path-spec, it can certainly matter.
>
> Personally, I think it's more interesting if this can make a difference 
> for something like
>
> 	git log v2.6.12.. -- drivers/ > /dev/null
>
> but that would require that we actually understand that we can stop early 
> if we ever get to REV_TREE_DIFFERENT. I didn't check if you actually did 
> that optimization.

The code is supposed to be there, but I haven't benched.

Re: [PATCH 2/5] diff --quiet

[Junio C Hamano]

Junio C Hamano <junkio@cox.net> writes:

> Linus Torvalds <torvalds@linux-foundation.org> writes:
>
>> Personally, I think it's more interesting if this can make a difference 
>> for something like
>>
>> 	git log v2.6.12.. -- drivers/ > /dev/null
>>
>> but that would require that we actually understand that we can stop early 
>> if we ever get to REV_TREE_DIFFERENT. I didn't check if you actually did 
>> that optimization.
>
> The code is supposed to be there, but I haven't benched.

Now I have.

In the kernel repository, I ran this with 'master' version and 'next'
version.  The latter uses the --quick mechanism in try_to_simplify.

$ /usr/bin/time git log -r --raw v2.6.19..master -- drivers/ | wc -l

Three runs on a reasonably quiescent machine (hot cache).

* next (i.e. with the --quick)

5.50user 0.10system 0:05.61elapsed 99%CPU (0avgtext+0avgdata 0maxresident)k
0inputs+0outputs (0major+15191minor)pagefaults 0swaps

5.50user 0.08system 0:05.58elapsed 99%CPU (0avgtext+0avgdata 0maxresident)k
0inputs+0outputs (0major+15191minor)pagefaults 0swaps

5.42user 0.07system 0:05.49elapsed 99%CPU (0avgtext+0avgdata 0maxresident)k
0inputs+0outputs (0major+15192minor)pagefaults 0swaps

* master (without)

7.50user 0.08system 0:07.59elapsed 99%CPU (0avgtext+0avgdata 0maxresident)k
0inputs+0outputs (0major+15487minor)pagefaults 0swaps

7.70user 0.06system 0:07.77elapsed 99%CPU (0avgtext+0avgdata 0maxresident)k
0inputs+0outputs (0major+15487minor)pagefaults 0swaps

7.51user 0.08system 0:07.60elapsed 99%CPU (0avgtext+0avgdata 0maxresident)k
0inputs+0outputs (0major+15487minor)pagefaults 0swaps

Re: [PATCH 2/5] diff --quiet

[Alex Riesen]

On 3/15/07, Junio C Hamano <junkio@cox.net> wrote:
> > Now I'm happy :)
> >
> > ~/linux$ time git diff-tree -r -s v2.6.16 v2.6.20
> >
> > real    0m0.137s
> > user    0m0.117s
> > sys     0m0.020s
> > ~/linux$ time ~/projects/git-diff/git-diff-tree -r --quiet v2.6.16 v2.6.20
> >
> > real    0m0.006s
> > user    0m0.000s
> > sys     0m0.007s
>
> You do not need diff-tree --quiet to do that!
>
>         $ git-rev-parse v2.6.16^{tree} v2.6.20^{tree}
>

Why would I want to benchmark --quiet with rev-parse?

Re: [PATCH 2/5] diff --quiet

[Alex Riesen]

On 3/15/07, Junio C Hamano <junkio@cox.net> wrote:
> "Alex Riesen" <raa.lkml@gmail.com> writes:
>
> > Actually, I find it a bit strange to leave the output silent,
> > if --quiet is turned off. For all the user sees, it is still quiet
> > and exit-coded
>
> For that exact reason, for all the user cares, it is better to
> leave the output turned off and result given back with exit
> status.  --quiet is really for scripted use, in other words.

Of course! That's also were the performance matters.
Can diff quit early if -Ssomething given?

> I wonder if we should perhaps turn off the PAGER when --exit-code
> or --quiet is given.
>

Second that.Even is just to be on safe side. But again, it is not obvious
in case of --exit-code.

Re: [PATCH 2/5] diff --quiet

[Johannes Schindelin]

Hi,

On Thu, 15 Mar 2007, Alex Riesen wrote:

> On 3/15/07, Junio C Hamano <junkio@cox.net> wrote:
> > > Now I'm happy :)
> > >
> > > ~/linux$ time git diff-tree -r -s v2.6.16 v2.6.20
> > >
> > > real    0m0.137s
> > > user    0m0.117s
> > > sys     0m0.020s
> > > ~/linux$ time ~/projects/git-diff/git-diff-tree -r --quiet v2.6.16
> > v2.6.20
> > >
> > > real    0m0.006s
> > > user    0m0.000s
> > > sys     0m0.007s
> > 
> > You do not need diff-tree --quiet to do that!
> > 
> >         $ git-rev-parse v2.6.16^{tree} v2.6.20^{tree}
> > 
> 
> Why would I want to benchmark --quiet with rev-parse?

It is not benchmarking, but it is a faster solution: you can see if two 
trees are different by comparing their SHA-1s.

(That, however, works only if you do not want something like "git diff 
-w"...)

Ciao,
Dscho

Re: [PATCH 2/5] diff --quiet

[Alex Riesen]

On 3/15/07, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> > >         $ git-rev-parse v2.6.16^{tree} v2.6.20^{tree}
> > >
> >
> > Why would I want to benchmark --quiet with rev-parse?
>
> It is not benchmarking, but it is a faster solution: you can see if two
> trees are different by comparing their SHA-1s.

Can the same be done for index? (index-tree comparison)

> (That, however, works only if you do not want something like "git diff
> -w"...)

Why? Can't "git diff -w" quit early?

Re: [PATCH 2/5] diff --quiet

[Linus Torvalds]

On Wed, 14 Mar 2007, Junio C Hamano wrote:
> >
> > The code is supposed to be there, but I haven't benched.
> 
> Now I have.

You da man!

> In the kernel repository, I ran this with 'master' version and 'next'
> version.  The latter uses the --quick mechanism in try_to_simplify.
> 
> $ /usr/bin/time git log -r --raw v2.6.19..master -- drivers/ | wc -l

> Three runs on a reasonably quiescent machine (hot cache).

Impressive, although not entirely surprising. "try_to_simplify()" is 
really the most performance-sensitive part of git.

I get a similar reduction for that load: 0:04.63 to 0:03.43 elapsed.

Good job.

		Linus

Re: [PATCH 2/5] diff --quiet

[Johannes Schindelin]

Hi,

On Thu, 15 Mar 2007, Alex Riesen wrote:

> On 3/15/07, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> > > >         $ git-rev-parse v2.6.16^{tree} v2.6.20^{tree}
> > > >
> > >
> > > Why would I want to benchmark --quiet with rev-parse?
> > 
> > It is not benchmarking, but it is a faster solution: you can see if two
> > trees are different by comparing their SHA-1s.
> 
> Can the same be done for index? (index-tree comparison)

Theoretically, yes. Only that Junio did not accept my short-cut ":dir/" 
notation to mean "take the cache-tree from the index, or if it is dirty, 
construct it". However, in the latter case, it would not be a speed 
improvement, but the opposite.

> > (That, however, works only if you do not want something like "git diff
> > -w"...)
> 
> Why? Can't "git diff -w" quit early?

No, but "-w" means "ignore white space", which means that blobs can be 
deemed equal, even if they differ at the byte-per-byte level.

Ciao,
Dscho

Re: [PATCH 2/5] diff --quiet

[Alex Riesen]

On 3/15/07, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> > > (That, however, works only if you do not want something like "git diff
> > > -w"...)
> >
> > Why? Can't "git diff -w" quit early?
>
> No, but "-w" means "ignore white space", which means that blobs can be
> deemed equal, even if they differ at the byte-per-byte level.
>

So it can leave early as soon as it found a difference on byte level
and the difference is not white space, can't it?

Re: [PATCH 2/5] diff --quiet

[Johannes Schindelin]

Hi,

On Thu, 15 Mar 2007, Alex Riesen wrote:

> On 3/15/07, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> > > > (That, however, works only if you do not want something like "git diff
> > > > -w"...)
> > >
> > > Why? Can't "git diff -w" quit early?
> > 
> > No, but "-w" means "ignore white space", which means that blobs can be

> > deemed equal, even if they differ at the byte-per-byte level.
> 
> So it can leave early as soon as it found a difference on byte level
> and the difference is not white space, can't it?

Yes.

The point I tried to make: without "-w" or "-b", you can compare at the 
tree level. No need for a --quiet option. (Of course, you showed me that 
you'd need it for index-tree comparisons anyway.)

Ciao,
Dscho

Re: [PATCH 2/5] diff --quiet

[Junio C Hamano]

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> The point I tried to make: without "-w" or "-b", you can compare at the 
> tree level. No need for a --quiet option.

That's not true.  The --quiet option is not about comparing at
the tree level vs looking inside the blobs.  It helps to
optimize the tree level comparison as well.  The difference
Linus was happy about comes from exactly the early exit from
tree comparison.

The current --quiet will report if they are different at the
tree level and does not take filtering done at the later phases
into account at all, and that is a conscious design decision so
that we can use it for optimizing try_to_simplify_commit().  So
for example, if you have any change that a plain "git diff-*"
says there are differences, the option reports there indeed are
differences, like this:

	$ git reset --hard ;# start from the clean state
        $ touch Makefile
        $ git diff-files -p
        diff --git a/Makefile b/Makefile
        $ git diff-files --quiet; echo $?
	1

I didn't consider -w nor -b when I made the design decision that
leads to the above behaviour, but I think the filtering done at
the textual patch generation phase falls into the same category.
If you do the following, the answer would be "they are
different":

	$ sed -e 's/$/ /' <Makefile >Makefile+
        $ cat Makefile+ >Makefile
        $ rm Makefile+
        $ git update-index --refresh
        $ git diff-files -b -p
        diff --git a/Makefile b/Makefile
        index dc024d4..1c328be 100644
        $ git diff-files -b -p --quiet; echo $?
        1

Options like -w and -b are only about not showing the lines; you
still get the diff header that reports they are different.  So
in that sense the output from the last command above is correct.

I am however wondering if we want to have (and if it would be
worth the trouble to add) an option to have the command say if
there will be textual difference output without producing it.  I
do not see much value in it right now, but it might turn out to
be handy.  I do suspect that would be a rather intrusive and
error prone change, though.

A good news is that such an option needs to work at a different
level than the existing --quiet, which works in diffcore_std().
The new option would probably need to work inside diff_flush()
which is a later phase.  So no matter how you screw up, it would
hopefully not break revision traversal too much.

We might want to rename the existing one --quick and name that
new option --quiet, though, if we go that route.