From: Santi Béjar <sbejar@gmail.com>
Date: Tue, 13 Mar 2007 23:14:50 GMT
Subject: Re: What's cooking in git.git (topics)
Message-ID: <8aa486160703131614i1b67e6c3vf7ccf395d63573b4@mail.gmail.com>
In-Reply-To: <7vhcsphqtk.fsf@assigned-by-dhcp.cox.net>

On 3/13/07, Junio C Hamano <junkio@cox.net> wrote:
> 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).

I think it is not fair, I wonder what would be the time with the merge
logic in sb/fetch in C. I'll try to make the git-fetch--tool
optimization.

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

I think my patch to support this is independent of the "Split fetch and merge".

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

Santi