Re: [External] Re: [PATCH 2/2] fetch-pack.c: do not declare local commits as "have" in partial repos

From: 韩仰 <hanyang.tony@bytedance.com>
Date: Sep 23, 2024, 03:44 UTC
Message-ID: <CAG1j3zHJVrpK5JZtUXFwkZgWY1-CxqET+ygpaMqo5aM-KeWaxg@mail.gmail.com>
In-Reply-To: <xmqqr09c89id.fsf@gitster.g>

On Sun, Sep 22, 2024 at 2:53 PM Junio C Hamano <gitster@pobox.com> wrote:

Show 7 quoted lines

> I was hoping to see that the issue can be fixed on the "gc" side,
> regardless of how the objects enter our repository, but perhaps I am
> missing something.  Isn't it just the matter of collecting C1, C3
> but not C2?  Or to put it another way, if we first create a list of
> objects to be packed (regardless of whether they are in promisor
> packs), and then remove the objects that are in promisor packs from
> the list, and pack the objects still remaining in the list?

I tried to fix the issue on the "gc" side following JTan's suggestion, by packing local objects referenced by promisor objects into promisor packs. But it turns out the cost for "for each promisor object, parse them and try to decide the objects they reference is in local repo" is too great. In a test blob:none partial clone repo, the gc would take more than one hour in the 2019 MacBook, despite the repo only having 17071073 objects. Normally it would take about 30 minutes.

> if we first create a list of
> objects to be packed (regardless of whether they are in promisor
> packs), and then remove the objects that are in promisor packs from
> the list, and pack the objects still remaining in the list?

This would work, though the remaining objects in the list would be suboptimally packed, due to the delta heuristic. Because we feed object id directly into git-pack-objects, instead of using rev-list. But that's how we pack promisor objects anyway.

In $JOB, we modified git-repack to pack everything into a giant promisor pack if the repo is partially cloned. This basically does the same thing as you suggested, but without the cost of constructing the object list and removing the objects in the promisor packs.

Thanks.