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

From: Junio C Hamano <gitster@pobox.com>
To: Jonathan Tan <jonathantanmy@google.com>,
	David Turner <dturner@twosigma.com>
Cc: stolee@gmail.com, git@vger.kernel.org
Subject: Re: [PATCH] cache-tree: do not lazy-fetch merge tree
Date: Wed, 04 Sep 2019 16:35:55 -0700	[thread overview]
Message-ID: <xmqq7e6nskh0.fsf@gitster-ct.c.googlers.com> (raw)
In-Reply-To: <20190904223529.135623-1-jonathantanmy@google.com> (Jonathan Tan's message of "Wed, 4 Sep 2019 15:35:29 -0700")

Jonathan Tan <jonathantanmy@google.com> writes:

> 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).

FWIW, this logic dates back to aecf567c ("cache-tree: create/update
cache-tree on checkout", 2014-07-05), when "checkout" learned to
perform opportunistic revalidation of cache-tree data structure,
without writing into the object store.  If we were lazily checked
out, and created a blob locally that happens to match the original
we did not fetch in a directory this piece of code is hashing, the
resulting hash _may_ name a tree that the other side has that we did
not fetch, so taking the "to_invalidate = 1" side would make the
resulting cache-tree less optimal, but because the design choice
being made here is to take that hit in order to avoid network cost,
as long as that is documented properly (iow, "probably doesn't have"
is not an issue; even if they have it, you do not want to fetch and
make the cache-tree entry valid), it is OK.