optimized checkout+rebase?

optimized checkout+rebase?

[Ralf Wildenhues]

Hello,

this is a common work pattern:

  git checkout master
  git pull
  for branch in $my_topic_branches; do
    git checkout $branch
    git rebase master
    # occasional fixups here...
  done

Now, it looks to me that one of the first operations of rebase just
undoes part of the work that the checkout of the branch did.  Well,
"undoes" is the wrong word, what I mean is that it looks like work
may be saved by combining the two checkout and the rewinding step.

Has this optimization already been implemented in git by some command
I've overlooked, and if no, do you agree that there is some optimization
possible?

Thanks,
Ralf

Re: optimized checkout+rebase?

[Johannes Sixt]

Ralf Wildenhues schrieb:
> this is a common work pattern:
> 
>   git checkout master
>   git pull
>   for branch in $my_topic_branches; do
>     git checkout $branch
>     git rebase master
>     # occasional fixups here...
>   done
> 
> Now, it looks to me that one of the first operations of rebase just
> undoes part of the work that the checkout of the branch did.  Well,
> "undoes" is the wrong word, what I mean is that it looks like work
> may be saved by combining the two checkout and the rewinding step.

Well, what you could do is:

   for branch in $my_topic_branches; do
     git rebase master $branch
     # occasional fixups here...
   done

But it seems that even then rebase first does the checkout $branch, and
then the checkout master right after that. At least the first checkout
should be unnecessary because all the revision range computations and
patch formating should be possible to do without the checkout. (The second
checkout is indispensable, of course.)

This has annoyed me, too, but not so much that I started looking at a fix,
though...

-- Hannes

Re: optimized checkout+rebase?

[Ralf Wildenhues]

* Johannes Sixt wrote on Thu, Mar 13, 2008 at 08:20:24AM CET:
> Well, what you could do is:
> 
>    for branch in $my_topic_branches; do
>      git rebase master $branch
>      # occasional fixups here...
>    done
> 
> But it seems that even then rebase first does the checkout $branch, and
> then the checkout master right after that.

Exactly.  The amount of typing is not an issue, but the trees are large
so it's not uncommon that each checkout takes tens of seconds.  No idea
how much of that is range computations and patch formatting though.

Thanks,
Ralf

[PATCH] rebase [--onto O] A B: omit needless checkout

[Junio C Hamano]

This teaches "git rebase [--onto O] A B" to omit an unnecessary checkout
of branch B before it goes on.

"git-rebase" originally was about rebasing the current branch to somewhere
else, and when the extra parameter to name which branch to rebase was
added, it defined the semantics to the safest but stupid "first switch to
the named branch and then operate exactly the same way as if we were
already on that branch".

But the first thing the real part of "rebase" does is to reset the work
tree and the index to the "onto" commit.  Which means the "rebase that
branch" form switched the work tree to the tip of the branch only to
immediately switch again to another commit.  This was wasteful.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---

 * Almost untested as I do not use this form very often.  Feedback would
   be good.

 git-rebase.sh |   51 +++++++++++++++++++++++++++++++--------------------
 1 files changed, 31 insertions(+), 20 deletions(-)

diff --git a/git-rebase.sh b/git-rebase.sh
index ff66af3..9273852 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -309,22 +309,41 @@ then
 	}
 fi
 
-# If the branch to rebase is given, first switch to it.
+# If the branch to rebase is given, that is the branch we will rebase
+# $branch_name -- branch being rebased, or HEAD (already detached)
+# $orig_head -- commit object name of tip of the branch before rebasing
+# $head_name -- refs/heads/<that-branch> or "detached HEAD"
+switch_to=
 case "$#" in
 2)
+	# Is it "rebase other $branchname" or "rebase other $commit"?
 	branch_name="$2"
-	git-checkout "$2" || usage
+	switch_to="$2"
+
+	if branch=$(git rev-parse --verify "refs/heads/$2" 2>/dev/null)
+	then
+		head_name="refs/heads/$2"
+	elif branch=$(git rev-parse --verify "$2" 2>/dev/null)
+	then
+		head_name="detached HEAD"
+	else
+		usage
+	fi
 	;;
 *)
+	# Do not need to switch branches, we are already on it.
 	if branch_name=`git symbolic-ref -q HEAD`
 	then
+		head_name=$branch_name
 		branch_name=`expr "z$branch_name" : 'zrefs/heads/\(.*\)'`
 	else
+		head_name="detached HEAD"
 		branch_name=HEAD ;# detached
 	fi
+	branch=$(git rev-parse --verify "${branch_name}^0") || exit
 	;;
 esac
-branch=$(git rev-parse --verify "${branch_name}^0") || exit
+orig_head=$branch
 
 # Now we are rebasing commits $upstream..$branch on top of $onto
 
@@ -335,6 +354,8 @@ if test "$upstream" = "$onto" && test "$mb" = "$onto" &&
 	# linear history?
 	! git rev-list --parents "$onto".."$branch" | grep " .* " > /dev/null
 then
+	# Lazily switch to the target branch if needed...
+	test -z "$switch_to" || git checkout "$switch_to"
 	echo >&2 "Current branch $branch_name is up to date."
 	exit 0
 fi
@@ -346,22 +367,11 @@ then
 	GIT_PAGER='' git diff --stat --summary "$mb" "$onto"
 fi
 
-# move to a detached HEAD
-orig_head=$(git rev-parse HEAD^0)
-head_name=$(git symbolic-ref HEAD 2> /dev/null)
-case "$head_name" in
-'')
-	head_name="detached HEAD"
-	;;
-*)
-	git checkout "$orig_head" > /dev/null 2>&1 ||
-		die "could not detach HEAD"
-	;;
-esac
-
-# Rewind the head to "$onto"; this saves our current head in ORIG_HEAD.
+# Detach HEAD and reset the tree
 echo "First, rewinding head to replay your work on top of it..."
-git-reset --hard "$onto"
+git checkout "$onto^0" >/dev/null 2>&1 ||
+	die "could not detach HEAD"
+# git reset --hard "$onto^0"
 
 # If the $onto is a proper descendant of the tip of the branch, then
 # we just fast forwarded.
@@ -374,7 +384,8 @@ fi
 
 if test -z "$do_merge"
 then
-	git format-patch -k --stdout --full-index --ignore-if-in-upstream "$upstream"..ORIG_HEAD |
+	git format-patch -k --stdout --full-index --ignore-if-in-upstream \
+		"$upstream..$orig_head" |
 	git am $git_am_opt --rebasing --resolvemsg="$RESOLVEMSG" &&
 	move_to_original_branch
 	ret=$?
@@ -397,7 +408,7 @@ echo "$orig_head" > "$dotest/orig-head"
 echo "$head_name" > "$dotest/head-name"
 
 msgnum=0
-for cmt in `git rev-list --reverse --no-merges "$upstream"..ORIG_HEAD`
+for cmt in `git rev-list --reverse --no-merges "$upstream..$orig_head"`
 do
 	msgnum=$(($msgnum + 1))
 	echo "$cmt" > "$dotest/cmt.$msgnum"
-- 
1.5.4.4.718.ga6ccf

Re: [PATCH] rebase [--onto O] A B: omit needless checkout

[Ralf Wildenhues]

Hello Junio,

* Junio C Hamano wrote on Sat, Mar 15, 2008 at 09:42:11PM CET:
> This teaches "git rebase [--onto O] A B" to omit an unnecessary checkout
> of branch B before it goes on.

>  * Almost untested as I do not use this form very often.  Feedback would
>    be good.

Nice, and seems to do exactly what I meant.  Actually the more important
speedup gained from this change is that files that have changed in the
non-common history of A but not in that of B, preserve their old time
stamps, so the rebuild of my bugfix topic B after having built master
is really quick now.

Thanks!
Ralf

Re: [PATCH] rebase [--onto O] A B: omit needless checkout

[Junio C Hamano]

Ralf Wildenhues <Ralf.Wildenhues@gmx.de> writes:

> Hello Junio,
>
> * Junio C Hamano wrote on Sat, Mar 15, 2008 at 09:42:11PM CET:
>> This teaches "git rebase [--onto O] A B" to omit an unnecessary checkout
>> of branch B before it goes on.
>
>>  * Almost untested as I do not use this form very often.  Feedback would
>>    be good.
>
> Nice, and seems to do exactly what I meant.  Actually the more important
> speedup gained from this change is that files that have changed in the
> non-common history of A but not in that of B, preserve their old time
> stamps, so the rebuild of my bugfix topic B after having built master
> is really quick now.

Actually, you would need this on top.  Otherwise

	$ git rebase master my-topic~3

would misbehave.

---

 git-rebase.sh |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/git-rebase.sh b/git-rebase.sh
index 9273852..bd55ef6 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -320,7 +320,8 @@ case "$#" in
 	branch_name="$2"
 	switch_to="$2"
 
-	if branch=$(git rev-parse --verify "refs/heads/$2" 2>/dev/null)
+	if git show-ref --verify --quiet -- "refs/heads/$2" &&
+	   branch=$(git rev-parse --verify "refs/heads/$2" 2>/dev/null)
 	then
 		head_name="refs/heads/$2"
 	elif branch=$(git rev-parse --verify "$2" 2>/dev/null)

Re: [PATCH] rebase [--onto O] A B: omit needless checkout

[Johannes Sixt]

Junio C Hamano schrieb:
> This teaches "git rebase [--onto O] A B" to omit an unnecessary checkout
> of branch B before it goes on.
> 
> "git-rebase" originally was about rebasing the current branch to somewhere
> else, and when the extra parameter to name which branch to rebase was
> added, it defined the semantics to the safest but stupid "first switch to
> the named branch and then operate exactly the same way as if we were
> already on that branch".
> 
> But the first thing the real part of "rebase" does is to reset the work
> tree and the index to the "onto" commit.  Which means the "rebase that
> branch" form switched the work tree to the tip of the branch only to
> immediately switch again to another commit.  This was wasteful.

This works for a frequent use-case of mine:

  $ git rebase master devel

where I am already on branch master, and now want to rebase devel on top
of it.

The code seems to take care of a lot of corner cases and less frequent
use-cases, including using --onto, which I don't feel able to judge about.

> +# git reset --hard "$onto^0"

Don't forget to remove this line. ;)

-- Hannes

Re: [PATCH] rebase [--onto O] A B: omit needless checkout

[Junio C Hamano]

Johannes Sixt <j.sixt@viscovery.net> writes:

> Junio C Hamano schrieb:
>> This teaches "git rebase [--onto O] A B" to omit an unnecessary checkout
>> of branch B before it goes on.
>> ...
>
> This works for a frequent use-case of mine:

"Works" meaning "does not break", or "gives a great performance
improvement that it is worth having it early in a tagged release"?

>   $ git rebase master devel
>
> where I am already on branch master, and now want to rebase devel on top
> of it.
>
> The code seems to take care of a lot of corner cases and less frequent
> use-cases, including using --onto, which I don't feel able to judge about.
>
>> +# git reset --hard "$onto^0"
>
> Don't forget to remove this line. ;)

Re: [PATCH] rebase [--onto O] A B: omit needless checkout

[Johannes Sixt]

Junio C Hamano schrieb:
> Johannes Sixt <j.sixt@viscovery.net> writes:
> 
>> Junio C Hamano schrieb:
>>> This teaches "git rebase [--onto O] A B" to omit an unnecessary checkout
>>> of branch B before it goes on.
>>> ...
>> This works for a frequent use-case of mine:
> 
> "Works" meaning "does not break", or "gives a great performance
> improvement that it is worth having it early in a tagged release"?

Sorry for being so terse. "Works", of course, means "works as expected",
and the "expected" part of that is that the annoyance is no longer
present, which is the unnecessary checkout.

But given how intrusive the patch is ("just to remove an unnecessary
checkout"), I'd say this is post-1.5.5 material. Also, the fact that you
had to post a fix-up is an indication that there are probably a number of
corner cases that need an extended testing period.

-- Hannes

Re: [PATCH] rebase [--onto O] A B: omit needless checkout

[Junio C Hamano]

Johannes Sixt <j.sixt@viscovery.net> writes:

> But given how intrusive the patch is ("just to remove an unnecessary
> checkout"), I'd say this is post-1.5.5 material. Also, the fact that you
> had to post a fix-up is an indication that there are probably a number of
> corner cases that need an extended testing period.

The fixup came from a real-world corner case.  I actually do fairly
esoteric varieties of rebases all the time, starting with my HEAD
detached, rebasing --onto a non-branch, explicitly stating where the
bottom is, etc, etc.

Although I am comfortable enough with the patch itself to queue it in
'next' for my daily use, I do not intend it for 1.5.5 at all.  The merge
window is a discipline, a line must be drawn somewhere, and when the line
is drawn, it must be honoured.