[PATCH] Use diff* with --exit-code in git-am, git-rebase and git-merge-ours

[PATCH] Use diff* with --exit-code in git-am, git-rebase and git-merge-ours

[Alex Riesen]

This simplifies the shell code and reduces memory footprint, speeds
things up. The performance improvements should be noticable by
git-rebase, when it is used to rebase big commits.

Signed-off-by: Alex Riesen <raa.lkml@gmail.com>
---
 git-am.sh         |   18 +++++++-----------
 git-merge-ours.sh |    2 +-
 git-rebase.sh     |   10 ++++------
 3 files changed, 12 insertions(+), 18 deletions(-)

diff --git a/git-am.sh b/git-am.sh
index 88af8dd..e9d8d57 100755
--- a/git-am.sh
+++ b/git-am.sh
@@ -408,12 +408,10 @@ do
 		# trust what the user has in the index file and the
 		# working tree.
 		resolved=
-		changed="$(git-diff-index --cached --name-only HEAD)"
-		if test '' = "$changed"
-		then
+		git-diff-index --quiet --exit-code --cached HEAD && {
 			echo "No changes - did you forget to use 'git add'?"
 			stop_here_user_resolve $this
-		fi
+		}
 		unmerged=$(git-ls-files -u)
 		if test -n "$unmerged"
 		then
@@ -435,13 +433,11 @@ do
 		then
 		    # Applying the patch to an earlier tree and merging the
 		    # result may have produced the same tree as ours.
-		    changed="$(git-diff-index --cached --name-only HEAD)"
-		    if test '' = "$changed"
-		    then
-			    echo No changes -- Patch already applied.
-			    go_next
-			    continue
-		    fi
+		    git-diff-index --quiet --exit-code --cached HEAD && {
+			echo No changes -- Patch already applied.
+			go_next
+			continue
+		    }
 		    # clear apply_status -- we have successfully merged.
 		    apply_status=0
 		fi
diff --git a/git-merge-ours.sh b/git-merge-ours.sh
index 4f3d053..40491f2 100755
--- a/git-merge-ours.sh
+++ b/git-merge-ours.sh
@@ -9,6 +9,6 @@
 # because the current index is what we will be committing as the
 # merge result.
 
-test "$(git-diff-index --cached --name-status HEAD)" = "" || exit 2
+git-diff-index --quiet --exit-code --cached HEAD || exit 2
 
 exit 0
diff --git a/git-rebase.sh b/git-rebase.sh
index aadd580..860c0ce 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -59,7 +59,7 @@ continue_merge () {
 		die "$RESOLVEMSG"
 	fi
 
-	if test -n "`git-diff-index HEAD`"
+	if ! git-diff-index --quiet --exit-code HEAD
 	then
 		if ! git-commit -C "`cat $dotest/current`"
 		then
@@ -124,13 +124,11 @@ while case "$#" in 0) break ;; esac
 do
 	case "$1" in
 	--continue)
-		diff=$(git-diff-files)
-		case "$diff" in
-		?*)	echo "You must edit all merge conflicts and then"
+		git-diff-files --quiet --exit-code || {
+			echo "You must edit all merge conflicts and then"
 			echo "mark them as resolved using git update-index"
 			exit 1
-			;;
-		esac
+		}
 		if test -d "$dotest"
 		then
 			prev_head="`cat $dotest/prev_head`"
-- 
1.5.1.rc1.63.g59cc5

Re: [PATCH] Use diff* with --exit-code in git-am, git-rebase and git-merge-ours

[Alex Riesen]

Alex Riesen, Sun, Mar 25, 2007 01:42:56 +0100:
> This simplifies the shell code and reduces memory footprint, speeds
> things up. The performance improvements should be noticable by
> git-rebase, when it is used to rebase big commits.

Please ignore, I forgot about "--quiet implies --exit-code" again.
Just documented the option. Will resend.

[PATCH] Use diff* with --exit-code in git-am, git-rebase and git-merge-ours

[Alex Riesen]

This simplifies the shell code, reduces its memory footprint, and
speeds things up. The performance improvements should be noticable
when git-rebase works on big commits.

Signed-off-by: Alex Riesen <raa.lkml@gmail.com>
---
 git-am.sh         |   18 +++++++-----------
 git-merge-ours.sh |    2 +-
 git-rebase.sh     |   10 ++++------
 3 files changed, 12 insertions(+), 18 deletions(-)

diff --git a/git-am.sh b/git-am.sh
index 88af8dd..e69ecbf 100755
--- a/git-am.sh
+++ b/git-am.sh
@@ -408,12 +408,10 @@ do
 		# trust what the user has in the index file and the
 		# working tree.
 		resolved=
-		changed="$(git-diff-index --cached --name-only HEAD)"
-		if test '' = "$changed"
-		then
+		git-diff-index --quiet --cached HEAD && {
 			echo "No changes - did you forget to use 'git add'?"
 			stop_here_user_resolve $this
-		fi
+		}
 		unmerged=$(git-ls-files -u)
 		if test -n "$unmerged"
 		then
@@ -435,13 +433,11 @@ do
 		then
 		    # Applying the patch to an earlier tree and merging the
 		    # result may have produced the same tree as ours.
-		    changed="$(git-diff-index --cached --name-only HEAD)"
-		    if test '' = "$changed"
-		    then
-			    echo No changes -- Patch already applied.
-			    go_next
-			    continue
-		    fi
+		    git-diff-index --quiet --cached HEAD && {
+			echo No changes -- Patch already applied.
+			go_next
+			continue
+		    }
 		    # clear apply_status -- we have successfully merged.
 		    apply_status=0
 		fi
diff --git a/git-merge-ours.sh b/git-merge-ours.sh
index 4f3d053..2b6a5c0 100755
--- a/git-merge-ours.sh
+++ b/git-merge-ours.sh
@@ -9,6 +9,6 @@
 # because the current index is what we will be committing as the
 # merge result.
 
-test "$(git-diff-index --cached --name-status HEAD)" = "" || exit 2
+git-diff-index --quiet --cached HEAD || exit 2
 
 exit 0
diff --git a/git-rebase.sh b/git-rebase.sh
index aadd580..1d96f32 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -59,7 +59,7 @@ continue_merge () {
 		die "$RESOLVEMSG"
 	fi
 
-	if test -n "`git-diff-index HEAD`"
+	if ! git-diff-index --quiet HEAD
 	then
 		if ! git-commit -C "`cat $dotest/current`"
 		then
@@ -124,13 +124,11 @@ while case "$#" in 0) break ;; esac
 do
 	case "$1" in
 	--continue)
-		diff=$(git-diff-files)
-		case "$diff" in
-		?*)	echo "You must edit all merge conflicts and then"
+		git-diff-files --quiet || {
+			echo "You must edit all merge conflicts and then"
 			echo "mark them as resolved using git update-index"
 			exit 1
-			;;
-		esac
+		}
 		if test -d "$dotest"
 		then
 			prev_head="`cat $dotest/prev_head`"
-- 
1.5.1.rc1.63.g59cc5

Re: [PATCH] Use diff* with --exit-code in git-am, git-rebase and git-merge-ours

[Junio C Hamano]

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

> This simplifies the shell code, reduces its memory footprint, and
> speeds things up. The performance improvements should be noticable
> when git-rebase works on big commits.

Thanks.

Unlike some optimizations that tilts the tradeoffs to favor
often-used case by penalizing the less often-used case, this
change does not penalize any case while optimizing some cases,
so there is nothing we are losing.  I'll take the patch for this
reason.

However, I have a few comments.

Have you run any benchmark?  I suspect "and speeds things up" is
a gross overstatement.  A qualified "SOMETIMES speeds things up"
would be a more honest thing to say.

Both of the changes to git-am are definitely good.  Under normal
usage, we expect that we would see some diff, and the optimization
applies to that normal case.

> diff --git a/git-merge-ours.sh b/git-merge-ours.sh
> index 4f3d053..2b6a5c0 100755
> --- a/git-merge-ours.sh
> +++ b/git-merge-ours.sh
> @@ -9,6 +9,6 @@
>  # because the current index is what we will be committing as the
>  # merge result.
>  
> -test "$(git-diff-index --cached --name-status HEAD)" = "" || exit 2
> +git-diff-index --quiet --cached HEAD || exit 2
>  
>  exit 0

I think this does not change the performance profile in the real
life.  When the user uses the command correctly, we expect that
index and HEAD to match, and in that case the diff would take
the same amount of processing with or without --quiet.  The
change does not penalize the opposite case, so it is not a wrong
thing to do, but this is an optimization to error out early.

The first change to git-rebase optimizes the case to handle
"still to be adopted upstream" case ("upstream swallowed our
change" case would perform the same as before), so the
optimization would be more visible by people with slower
upstream and/or people patches of lessor quality that are
rejected by upstream often.  In other words, the amount of
speeding up by this change really depends on the user (there is
no slowdown for anybody so that is Ok).

The other optimizes the "erroring out early" uninteresting case.

Re: [PATCH] Use diff* with --exit-code in git-am, git-rebase and git-merge-ours

[Alex Riesen]

Junio C Hamano, Sun, Mar 25, 2007 08:01:31 +0200:
> However, I have a few comments.
> 
> Have you run any benchmark?  I suspect "and speeds things up" is

I didn't benchmark any of these, so I can't backup the statement with
the number but...

> a gross overstatement.  A qualified "SOMETIMES speeds things up"
> would be a more honest thing to say.

it must speed things up, if only for removed shell-side efforts to
redirect the output and store it in heap. To overstatement it I should
have said: "greatly speeds things up".