Re: [PATCH] packfile: skip decompressing and hashing blobs in add_promisor_object()
- From
Jeff King <peff@peff.net>
- Date
- Dec 5, 2025, 17:48 UTC
- Message-ID
- <20251205174854.GA18566@coredump.intra.peff.net>
- In-Reply-To
- <20251204172132.319360-1-aplattner@nvidia.com>
On Thu, Dec 04, 2025 at 09:21:29AM -0800, Aaron Plattner wrote:
Show 9 quoted lines
> For repositories with large pack files, this can take an extremely long > time. For example, on a production repository with a 176 GB promisor > pack: > > $ time ~/git/git/git-rev-list --objects --all --exclude-promisor-objects --quiet > ________________________________________________________ > Executed in 76.10 mins fish external > usr time 72.10 mins 1.83 millis 72.10 mins > sys time 3.56 mins 0.17 millis 3.56 mins
FWIW, I had a hard time replicating the results with this command, because it won't necessarily call is_promisor_object(). It only does so when it finds a missing object. But it also marks everything in the promisor pack as UNINTERESTING from the start, so you need a non-promisor commit that points to an object excluded from the narrow clone.
An easier way to trigger it is with a fake oid like:
rev-list --objects --all --exclude-promisor-objects $(perl -e 'print "1" x 40')
Then we have to check is_promisor_object() to know that the 111... oid isn't really a promisor mentioned somewhere.
Show 10 quoted lines
> For objects that weren't already parsed, use odb_read_object_info() to > query the object type. If it's a blob, just insert it into the oidset > without parsing it. This improves performance for very large pack files > significantly: > > $ time ~/git/git/git-rev-list --objects --all --exclude-promisor-objects --quiet > ________________________________________________________ > Executed in 118.76 secs fish external > usr time 50.88 secs 11.02 millis 50.87 secs > sys time 36.31 secs 0.08 millis 36.31 secs
Yeah, this is obviously a good idea. This all seemed eerily familiar, and I wondered if we weren't doing this already. But it looks like it came up as "maybe we should do this" along with some other optimizations, but we never did it. Your 176GB (!) repository is obviously a good way to show off the change. But I think we can see it even in a fresh clone of linux.git, which (with my command above) goes from ~7.5 minutes to under 2 minutes with your patch.
But I have an idea that makes your patch a little simpler and should give us another little speed bump.
Show 19 quoted lines
> diff --git a/packfile.c b/packfile.c
> index 9cc11b6dc5..563fd14f0e 100644
> --- a/packfile.c
> +++ b/packfile.c
> @@ -2309,6 +2309,17 @@ static int add_promisor_object(const struct object_id *oid,
> if (obj && obj->parsed) {
> we_parsed_object = 0;
> } else {
> + /*
> + * Blobs don't reference other objects, so skip parsing them
> + * to save time.
> + */
> + enum object_type type;
> + type = odb_read_object_info(pack->repo->objects, oid, NULL);
> + if (type == OBJ_BLOB) {
> + oidset_insert(set, oid);
> + return 0;
> + }
> +OK, so we are checking the type up front and then skipping parse_object() if we can. But there is already some logic inside parse_object() for these kinds of optimizations. If we tell it we are not interested in checking the hash of the objects, then it knows it can skip loading the blob entirely.
But it can _also_ use that flag for other things, like using the commit-graph rather than loading individual commit objects. So doing this:
diff --git a/packfile.c b/packfile.c index 9cc11b6dc5..01b992a4e1 100644 --- a/packfile.c +++ b/packfile.c @@ -2310,7 +2310,8 @@ static int add_promisor_object(const struct object_id *oid, we_parsed_object = 0; } else { we_parsed_object = 1; - obj = parse_object(pack->repo, oid); + obj = parse_object_with_flags(pack->repo, oid, + PARSE_OBJECT_SKIP_HASH_CHECK); } if (!obj) drops my linux.git case down to 49s. It's skipping the blobs (with no need for your patch) and loading the commits out of the graph file. Note that you may need to "git commit-graph write --reachable" to see the effect (I think we do generate graphs by default in git-gc these days, but I'm not sure if we do so right after cloning). -Peff