On Wed, Jun 15, 2022 at 02:17:58PM -0400, Derrick Stolee wrote: > 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. I agree that tree misses are awful, but I'm actually talking about something different: traversing the local trees we _do_ have in order to find the set of promised objects. Which is worse for blob:none, because it means you have more trees locally. :) Try this with a big repo like linux.git: git clone --no-local --filter=blob:none linux.git repo cd repo # this is fast; we mark the promisor trees as UNINTERESTING, so we do # not look at them as part of the traversal, and never call # is_promisor_object(). time git rev-list --count --objects --all --exclude-promisor-objects # but imagine we had a fixed mktree[1] that did not fault in the blobs # unnecessarily, and we made a new tree that references a promised # blob. tree=$(git ls-tree HEAD~1000 | grep Makefile | git mktree --missing) commit=$(echo foo | git commit-tree -p HEAD $tree) git update-ref refs/heads/foo $commit # this is now slow; even though we only call is_promisor_object() # once, we have to open every single tree in the pack to find it! time git rev-list --count --objects --all --exclude-promisor-objects Those rev-lists run in 1.7s and 224s respectively. Ouch! Now the setup there is kind of contrived, and it's actually not trivial to convince rev-list to actually call is_promisor_object() these days. That's because promisor-ness (promisosity?) tends to be one-way transitive. A promised blob is usually either only referenced by promised trees (which we have in this case), or is faulted in as part of making a new tree. But it's not guaranteed, and certainly a faulted-in object could later be deleted from the local repo, since it's promised. I suspect there are less convoluted workflows to get to a similar state, but I don't know them offhand. There's more discussion of this issue in this thread from last summer: https://lore.kernel.org/git/20210403090412.GH2271@xxxxxxxxxx/ -Peff [1] The mktree I used was the fix discussed elsewhere in the thread: diff --git a/builtin/mktree.c b/builtin/mktree.c index 902edba6d2..42ae82230c 100644 --- a/builtin/mktree.c +++ b/builtin/mktree.c @@ -117,15 +117,11 @@ static void mktree_line(char *buf, int nul_term_line, int allow_missing) } /* Check the type of object identified by sha1 */ - obj_type = oid_object_info(the_repository, &oid, NULL); - if (obj_type < 0) { - if (allow_missing) { - ; /* no problem - missing objects are presumed to be of the right type */ - } else { + if (!allow_missing) { + obj_type = oid_object_info(the_repository, &oid, NULL); + if (obj_type < 0) die("entry '%s' object %s is unavailable", path, oid_to_hex(&oid)); - } - } else { - if (obj_type != mode_type) { + else if (obj_type != mode_type) { /* * The object exists but is of the wrong type. * This is a problem regardless of allow_missing
