Re: [PATCH] repack-promisor: add fake paths to oids when repacking promisor objects

From: Junio C Hamano <gitster@pobox.com>
Date: Mar 31, 2026, 15:51 UTC
Message-ID: <xmqqqzp02e1h.fsf@gitster.g>
In-Reply-To: <actjaxIkDEXHJbyi@pks.im>

Patrick Steinhardt <ps@pks.im> writes:

Show 13 quoted lines

>> This will ensure they can be grouped by the type and existing pack
>> order which will make them end up close together in the sort, improving
>> delta compression.
>
> I think the general idea may be sound, but ideally we would have some
> benchmarks that demonstrate it actually is. Like, can you come up with
> scenarios where it will indeed improve the packfile size and show the
> advantage of this change? Are there scenarios that are likely to have a
> disadvantage because of this new ordering? Which of these scenarios do
> we expect to be more likely?
>
> Before answering these questions we basically just claim it's going to
> be an improvement without actually verifying.

While it is a very good point that a change that claims to improve performance must come with verifyable data, because the packfile size alone is not what you want to optimize for in the first place, it is quite hard to come up with a useful benchmark in this area.

Back when I was working on packfile generation, we needed to optimize for two things (luckily they are not competing goals). One is to choose a good delta-base, which will contribute to an overall pack size that is smaller. The ordering of the objects in a pack, on the other hand, does not directly contribute to the size, but has impact on runtime performance, by keeping related things closer together to reduce the need to "seek" in the pack stream.

Generally, two objects that appear next to each other in a well optimized packstream are not expected to be similar with each other. They are more likely to be two unrelated files that appear in the same tree object (i.e., they do not delta with each other well, but at runtime, they are often needed together). So it may even be detrimental to use the offset in packfile as a clue to choose among potential delta bases.

Do we have name-hash data for the original pack somewhere available so that the repacker can take advantage of? If so, it may be more relevant thing to reuse.

I agree with your other points in your review, too. Thanks for helping this topic.