Re: What's cooking in git.git (topics)

[Junio C Hamano <junkio@cox.net>]

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

> Here are the topics that have been cooking.

> * sb/fetch (Mon Mar 12 19:01:11 2007 -0700) 19 commits
>  + git-fetch.sh:append_fetch_head() no longer has a remote_nick
>    argument
>  + git-fetch: Split fetch and merge logic
>
> I have a soft spot to anything that claims to be a clean-up, but
> I suspect that the shell loop this series introduces may defeat
> the git-fetch--tool optimization.  Also I think having to base
> the patch on this made Paolo's "dot is special token to mean
> 'git pull' merges from a local branch" needlessly complex (but I
> haven't tried rewriting it myself without these two).  Although
> I merged these to 'next', I am considering to revert them.

I tried the "NULL fetch between 1000-refs repositories" test,
which prompted the git-fetch--tool work that was done on
jc/fetch topic in 'next', with the following versions:

 (1) 1.5.0 (without any git-fetch--tool optimization)
 (2) master (ditto)
 (3) master with jc/fetch (but not sb/fetch topic)
 (4) next ((3) plus sb/fetch and others)

The test scripts are at the end of this message.  Both (1) and
(2) take 3 minutes 7 seconds wallclock time.  (3) improves it
down to 15 seconds.  (4) makes the operation spend 24 seconds
(the times are all on my primary machine x86-64 with 1GB, hot
cache and average of three runs each).

So the "Split fetch and merge" series hurts the performance
quite a bit.  If it had enough "code clean-up" merit to warrant
this, I would say it probably is a cost we should bear, but I
personally do not see it.

Paolo recently worked on top of next to base the fake '.' remote
patch.  This wants to allow:

	[branch "foo"]
        	remote = .
                merge = refs/heads/master

with an implicit (meaning, you do not have to have this in your
configuration):

	[remote "."]
        	url = .
                fetch = refs/*

so that you can say:

	$ git checkout foo
        $ git pull

to merge from the local 'master' branch.

I haven't reimplemented Paolo's patch on top of (3) above for
comparison, but I have a feeling that it would not have been
helped by the alleged clean-up value of "Split fetch and merge"
patch (iow, I do not think it would be the case that the code
got clearer to understand thanks to the clean-up).

What Paolo's patch needs to do is to bypass the actual fetch and
generate the following line in .git/FETCH_HEAD:

	sha1-of-our-master <TAB> <TAB> branch 'master' of .

I even suspect that "Split fetch and merge", by introducing
FETCH_FETCHED and making FETCH_HEAD generated from it, made
Paolo's patch more difficult to do and the end result less
efficient.

So unless there is a convincing counterexample otherwise, I'd
like to revert the "Split fetch and merge" series.


-- >8 -- setting up test repositories -- >8 --
#!/bin/sh

rm -fr origin clone

mkdir origin
cd origin
git init
: >hello
git add hello
git commit -a -m 'initial'
i=0
while test $i -lt 500
do
	git tag t$i
	git branch b$i
	i=$(($i+1))
done

: >bye
git add bye
git commit -a -m 'second'
while test $i -lt 1000
do
	git tag t$i
	git branch b$i
	i=$(($i+1))
done

cd ..
-- >8 -- NULL fetch test -- >8 --
#!/bin/sh

cd clone
echo '* fetching'
time git fetch origin