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

Previous thread: [PATCH 1/5] Remove unused diffcore_std_no_resolve by Junio C Hamano on Wednesday, March 14, 2007 - 2:26 pm. (1 message)

Next thread: [PATCH 5/5] try-to-simplify-commit: use diff-tree --quiet machinery. by Junio C Hamano on Wednesday, March 14, 2007 - 2:26 pm. (1 message)
From: Junio C Hamano
Date: Wednesday, March 14, 2007 - 2:26 pm

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 ...
From: Alex Riesen
Date: Wednesday, March 14, 2007 - 3:33 pm

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.
-

From: Alex Riesen
Date: Wednesday, March 14, 2007 - 3:54 pm

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?)
-

From: Junio C Hamano
Date: Wednesday, March 14, 2007 - 4:37 pm

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.

-

From: Alex Riesen
Date: Thursday, March 15, 2007 - 1:22 am

Of course! That's also were the performance matters.

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

From: Alex Riesen
Date: Wednesday, March 14, 2007 - 4:14 pm

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
-

From: Junio C Hamano
Date: Wednesday, March 14, 2007 - 4:53 pm

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

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


-

From: Linus Torvalds
Date: Wednesday, March 14, 2007 - 5:33 pm

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
-

From: Junio C Hamano
Date: Wednesday, March 14, 2007 - 6:23 pm

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

-

From: Junio C Hamano
Date: Wednesday, March 14, 2007 - 11:47 pm

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


-

From: Linus Torvalds
Date: Thursday, March 15, 2007 - 9:08 am

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
-

From: Alex Riesen
Date: Thursday, March 15, 2007 - 1:19 am

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

From: Johannes Schindelin
Date: Thursday, March 15, 2007 - 3:37 am

Hi,


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

-

From: Alex Riesen
Date: Thursday, March 15, 2007 - 6:55 am

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

From: Johannes Schindelin
Date: Thursday, March 15, 2007 - 10:43 am

Hi,


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 

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

-

From: Alex Riesen
Date: Thursday, March 15, 2007 - 2:08 pm

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?
-

From: Johannes Schindelin
Date: Thursday, March 15, 2007 - 2:12 pm

Hi,


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

-

Previous thread: [PATCH 1/5] Remove unused diffcore_std_no_resolve by Junio C Hamano on Wednesday, March 14, 2007 - 2:26 pm. (1 message)

Next thread: [PATCH 5/5] try-to-simplify-commit: use diff-tree --quiet machinery. by Junio C Hamano on Wednesday, March 14, 2007 - 2:26 pm. (1 message)