Re: [PATCH] mktree: learn about promised objects

From: Derrick Stolee <derrickstolee@github.com>
Date: Jun 15, 2022, 18:17 UTC
Message-ID: <1fe6c00a-806c-89de-cb67-d063dc4a5279@github.com>
In-Reply-To: <ad9b5ec9-14fd-cd66-be87-2fe1eb24296a@roku.com>

On 6/15/2022 1:40 PM, Richard Oliver wrote:

> On 15/06/2022 05:00, Jeff King wrote:

>> So it is not just lookup, but actual tree walking that is expensive. The
>> flip side is that you don't have to store a complete separate list of
>> the promised objects. Whether that's a win depends on how many local
>> objects you have, versus how many are promised.

This is also why blobless (or blob-size filters) are the recommended way to use partial clone. It's just too expensive to have tree misses.

Show 18 quoted lines

>> But it would be possible to cache the promisor list to make the tradeoff
>> separately. E.g., do the walk over the promisor trees once (perhaps at
>> pack creation time), and store a sorted list of fixed-length (oid, type)
>> records that could be binary searched. You could even put it in the
>> .promisor file. :)
>>
>> -Peff
> 
> I like the idea of caching the promisor list at pack creation time;
> I'll start work on a patch set that implements this.
> 
> Meanwhile, is it worth considering a '--promised-as-missing' option
> (or a config option) for invocations such as 'mktree --missing' that
> prevents promised objects being faulted-in? Currently, the only
> reliable way that I've found to prevent 'mktree --missing' faulting-in
> promised objects is to remove the remote. Such an option could either
> set the global variable 'fetch_if_missing' to '0' or could ensure
> 'OBJECT_INFO_SKIP_FETCH_OBJECT' is passed appropriately.

One issue I've had with the current design of partial clone is that we put all of the filter logic into the remotes, not the repository itself. This means that if I use "git remote add" to add a new remote, then that remote does not inherit the filter (and hence fetches can be too large or even fail because we are "missing" an object pointed to by the resulting pack).

If we had a repository-scoped notion of the filter, then we could special-case this mktree logic to assume a blob if the filter only excludes blobs. That would be even faster than looking up the type from a "promised objects" file.

Just thinking about smaller steps that could simplify things before adding a new data format. Feel free to continue pursuing that lookup file if that's what you think will work best.

Thanks, -Stolee