Re: [PATCH v2 2/2] pack-objects: only perform verbatim reuse on the preferred pack

From: Jeff King <peff@peff.net>
Date: Nov 22, 2024, 08:33 UTC
Message-ID: <20241122083350.GD1179306@coredump.intra.peff.net>
In-Reply-To: <xmqqjzcvand6.fsf@gitster.g>

On Fri, Nov 22, 2024 at 01:44:37PM +0900, Junio C Hamano wrote:

Show 16 quoted lines

> Taylor Blau <me@ttaylorr.com> writes:
> 
> > This regression comes from ca0fd69e37 (pack-objects: prepare
> > `write_reused_pack_verbatim()` for multi-pack reuse, 2023-12-14), which
> > incorrectly assumed that there would be no gaps in reusable regions of
> > non-preferred packs.
> 
> I was reviewing the past release notes from this year, and saw this
> snippet in the 2.44 release notes:
> 
>  * Streaming spans of packfile data used to be done only from a
>    single, primary, pack in a repository with multiple packfiles.  It
>    has been extended to allow reuse from other packfiles, too.
> 
> Essentially this two-patch series the change completely so we only
> reuse pack data from the primary pack?

No. There are really three levels in increasing order of cost:

  1. we directly copy out the bytes from the start of a pack completely,
     up until the first object that is not needed to be sent

  2. from there we process objects one by one, with a few checks and
     adjustments to make sure they can be copied verbatim (like ofs
     delta adjustment to account for earlier missing objects)

  3. we add the objects to the big object_list struct, they become
     candidates for delta compression, etc

We call the first two generically "pack reuse". And this two-patch series is fixing a case where we were doing (1), but needed to do (2).

So I think the changelog entry still applies (there are other cases where we might jump from 1 to 2 as well, like if the output needs to convert to REF_DELTA).

-Peff