Re: [PATCH 2/2] index-pack: prefetch missing REF_DELTA bases

From: Johannes Schindelin <Johannes.Schindelin@gmx.de>
To: Jonathan Tan <jonathantanmy@google.com>
Cc: git@vger.kernel.org
Subject: Re: [PATCH 2/2] index-pack: prefetch missing REF_DELTA bases
Date: Fri, 17 May 2019 20:33:45 +0200 (DST)	[thread overview]
Message-ID: <nycvar.QRO.7.76.6.1905172032550.46@tvgsbejvaqbjf.bet> (raw)
In-Reply-To: <20190515182812.107420-1-jonathantanmy@google.com>

Hi Jonathan,

On Wed, 15 May 2019, Jonathan Tan wrote:

> > > To resolve this, prefetch all missing REF_DELTA bases before attempting
> > > to resolve them. This both ensures that all bases are attempted to be
> > > fetched, and ensures that we make only one request per index-pack
> > > invocation, and not one request per missing object.
> >
> > Hmm. I wonder whether this can lead to *really* undesirable behavior, e.g.
> > with deep delta chains. The client would possibly have to fetch the
> > REF_DELTA object, but that would also be delivered in a thin pack with
> > *another* REF_DELTA object, and the same over and over again, with plenty
> > of round trips that kill performance really well.
>
> When the client fetches the REF_DELTA base, it won't be a REF_DELTA
> object itself because Git makes these fetches without any "have" lines,
> so the server doesn't know anything to delta against. Admittedly, this
> is just due how to we implemented it - if later we find a way to
> optimize the lazy fetches by adding "have", then we'll have to revisit
> this.

Ah! I *think* I understand this better now. Thank you.

> Quoting from the commit message:
>
> > > (When fetching REF_DELTA bases, it is unlikely that
> > > those are REF_DELTA themselves, because we do not send "have" when
> > > making such fetches.)
>
> I tried to address this point with this sentence in the commit message.
> If you think that this should be addressed more clearly in the commit
> message, let me know if you have any suggestions.

I totally read over this part of the commit message, apparently. My bad.

Sorry for the noise!
Dscho