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

From: Jeff King <peff@peff.net>
Date: Sep 5, 2019, 07:01 UTC
Message-ID: <20190905070153.GE21450@sigill.intra.peff.net>
In-Reply-To: <20190826214737.164132-1-jonathantanmy@google.com>

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

Show 11 quoted lines

> 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