Re: [PATCH] cache-tree: do not lazy-fetch merge tree

From: Jonathan Tan <jonathantanmy@google.com>
Date: Sep 4, 2019, 22:35 UTC
Message-ID: <20190904223529.135623-1-jonathantanmy@google.com>
In-Reply-To: <69f2b069-3e11-4a2e-9b81-bde18c463d8c@gmail.com>

Show 11 quoted lines

> On 9/3/2019 3:42 PM, Jonathan Tan wrote:
> > When cherry-picking (for example), new trees may be constructed. During
> > this process, Git checks whether these trees exist. However, in a
> > partial clone, this causes a lazy fetch to occur, which is both
> > unnecessary (because Git has already constructed this tree as part of
> > the cherry-picking process) and likely to fail (because the remote
> > probably doesn't have this tree).
> 
> If we have constructed the object already, then why do we not see it
> and avoid fetching it? This must be a slightly strange timing issue
> with objects being flushed to disk or added to the object cache.

Thanks for taking a look at this! The answer is that I wasn't precise when I said "already constructed" - I meant that it is in a struct strbuf. It hasn't been written to disk yet, so has_object_file() does not see it.

> One approach is to find all of these has_object_file() calls that should
> really be one with OBJECT_INFO_SKIP_FETCH_OBJECT. Another would be to
> find out why has_object_file() isn't seeing the object we constructed.

By the former, do you mean that we should look at the other has_object_file() calls? I looked at the others in cache-tree.c and I think the one in this patch is the only one that is called on an OID generated from hash_object_file(). (And I answered the latter in the above paragraph.)

To avoid confusion, maybe this commit message is better:

When cherry-picking (for example), new trees may be constructed. During this process, Git constructs the new tree in a struct strbuf, computes the OID of the new tree, and checks if the new OID already exists on disk. However, in a partial clone, the disk check causes a lazy fetch to occur, which is both unnecessary (because we have the tree in the struct strbuf) and likely to fail (because the remote probably doesn't have this tree).