Re: [PATCH] fetch-pack: write fetched refs to .promisor

From: Jeff King <peff@peff.net>
To: Jonathan Tan <jonathantanmy@google.com>
Cc: git@vger.kernel.org
Subject: Re: [PATCH] fetch-pack: write fetched refs to .promisor
Date: Thu, 5 Sep 2019 03:01:53 -0400	[thread overview]
Message-ID: <20190905070153.GE21450@sigill.intra.peff.net> (raw)
In-Reply-To: <20190826214737.164132-1-jonathantanmy@google.com>

On Mon, Aug 26, 2019 at 02:47:37PM -0700, Jonathan Tan wrote:

> The specification of promisor packfiles (in partial-clone.txt) states
> that the .promisor files that accompany packfiles do not matter (just
> like .keep files), so whenever a packfile is fetched from the promisor
> remote, Git has been writing empty .promisor files. But these files
> could contain more useful information.
> 
> So instead of writing empty files, write the refs fetched to these
> files. This makes it easier to debug issues with partial clones, as we
> can identify what refs (and their associated hashes) were fetched at the
> time the packfile was downloaded, and if necessary, compare those hashes
> against what the promisor remote reports now.

I'm not really opposed to what you're doing here, but I did recently
think of another possible use for .promisor files. So it seems like a
good time to bring it up, since presumably we'd have to choose one or
the other.

I noticed when playing with partial clones that the client may sometimes
pause for a while, chewing CPU. The culprit is is_promisor_object(),
which generates the list of known promisor objects by opening every
object we _do_ have to find out which ones they mention.

I know one of the original design features of the promisor pack was that
the client would _not_ keep a list of all of the objects it didn't have.
But I wonder if it would make sense to keep a cache of these "cut
points" in the partial clone. That's potentially smaller than the
complete set of objects (especially for tree-based partial cloning), and
it seems clear we're willing to store it in memory anyway.

And if we do that, would the .promisor file for a pack be a good place
to store it?

-Peff