All of lore.kernel.org
 help / color / mirror / Atom feed
From: Johan Herland <johan@herland.net>
To: git@vger.kernel.org
Cc: Nicolas Pitre <nico@cam.org>, Junio C Hamano <gitster@pobox.com>,
	Daniel Barkalow <barkalow@iabervon.org>,
	Johannes Schindelin <Johannes.Schindelin@gmx.de>
Subject: Re: [PATCH] fix simple deepening of a repo
Date: Mon, 24 Aug 2009 16:20:52 +0200	[thread overview]
Message-ID: <200908241620.52838.johan@herland.net> (raw)
In-Reply-To: <alpine.LFD.2.00.0908240946390.6044@xanadu.home>

On Monday 24 August 2009, Nicolas Pitre wrote:
> On Sun, 23 Aug 2009, Junio C Hamano wrote:
> > Nicolas Pitre <nico@cam.org> writes:
> > > If all refs sent by the remote repo during a fetch are reachable
> > > locally, then no further conversation is performed with the
> > > remote. This check is skipped when the --depth argument is
> > > provided to allow the deepening of a shallow clone which
> > > corresponding remote repo has no changed.
> > >
> > > However, some additional filtering was added in commit c29727d5
> > > to remove those refs which are equal on both sides.  If the
> > > remote repo has not changed, then the list of refs to give the
> > > remote process becomes empty and simply attempting to deepen a
> > > shallow repo always fails.
> > >
> > > Let's stop being smart in that case and simply send the whole
> > > list over when that condition is met.  The remote will do the
> > > right thing anyways.
> > >
> > > Test cases for this issue are also provided.
> > >
> > > Signed-off-by: Nicolas Pitre <nico@cam.org>
> > > ---
> >
> > Thanks.  The fix looks correct (as usual with patches from you).
> >
> > But it makes me wonder if this logic to filter refs is buying us
> > anything.
> >
> > >  	for (rm = refs; rm; rm = rm->next) {
> > > +		nr_refs++;
> > >  		if (rm->peer_ref &&
> > >  		    !hashcmp(rm->peer_ref->old_sha1, rm->old_sha1))
> > >  			continue;
> >
> > 		ALLOC_GROW(heads, nr_heads + 1, nr_alloc);
> > 		heads[nr_heads++] = rm;
> > 	}
> >
> > What is the point of not asking for the refs that we know are the
> > same?
>
> I could see the advantage if the number of refs is really huge. 
> Wasn't some converted repositories producing a lot of branches and/or
> tags significantly slowing down a fetch operation?  Granted that was
> long ago when that issue got "fixed" so the details are fuzzy to me.

I'm converting several CVS repos to Git with ~50 000 refs, so I'm happy 
with any change that can speed things up for repos with many refs.

Right now, my biggest gripe is that a 'git push --mirror' on such a repo 
can easily take ~10 min. even though the actual pack generation and 
transfer only takes a couple of seconds. It seems like it needs ~10 
minutes to generate the list of changed/added/deleted refs...
Unfortunately I haven't had time to look properly into it, yet...


...Johan

-- 
Johan Herland, <johan@herland.net>
www.herland.net

  reply	other threads:[~2009-08-24 14:21 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-08-22  5:52 git fetch --depth=* broken? Nicolas Pitre
2009-08-24  4:04 ` [PATCH] fix simple deepening of a repo Nicolas Pitre
2009-08-24  4:49   ` Junio C Hamano
2009-08-24 13:55     ` Nicolas Pitre
2009-08-24 14:20       ` Johan Herland [this message]
2009-08-24 22:21       ` Junio C Hamano
2009-08-24 16:26     ` Daniel Barkalow
2009-08-24 22:30       ` Julian Phillips
2009-08-25  0:18         ` Nicolas Pitre
2009-08-25  2:12           ` Shawn O. Pearce
2009-08-25  5:00             ` Sverre Rabbelier
2009-08-25  5:21             ` Junio C Hamano
2009-08-25  6:12               ` Shawn O. Pearce
2009-08-25  6:33                 ` Junio C Hamano
2009-08-25 15:14                   ` Shawn O. Pearce
2009-08-26  2:10                     ` Shawn O. Pearce
2009-08-26  7:08                       ` Johannes Sixt
2009-08-26  8:22                         ` Shawn O. Pearce
2009-08-26  9:03                           ` Junio C Hamano
2009-08-26 17:03                             ` Shawn O. Pearce
2009-08-28 17:30                       ` [RFC PATCH] upload-pack: expand capability advertises additional refs Shawn O. Pearce
2009-08-28 19:07                         ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=200908241620.52838.johan@herland.net \
    --to=johan@herland.net \
    --cc=Johannes.Schindelin@gmx.de \
    --cc=barkalow@iabervon.org \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=nico@cam.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.