Re: [PATCH v3 0/8] pack-objects: Create an alternative name hash algorithm (recreated)

[Taylor Blau <me@ttaylorr.com>]

On Tue, Jan 21, 2025 at 03:21:15PM -0500, Derrick Stolee wrote:
> This series has been at this version for a while. I'm pretty sure that this
> is the most promising direction we have at the moment for improving delta
> compression for many users.
>
> The only decision point I think remains is whether or not to include the last
> patch (--name-hash-version=3) which I would be happy either way.

Sorry that I punted on reviewing this for way longer than I should have,
and thanks for bearing with me.

I took a close look at this latest round of patches, skipping over the
parts that I remembered from previous rounds. My memory is far from
perfect, so I may have commented on things that we've already discussed,
in which case I apologize :-).

I left a handful of comments on the patches themselves, but they are
mostly cosmetic. My idle thought before having a chance to review this
series is that the --name-hash-version option was handing over too much
control to the user without clear instruction on when to use one version
over the other.

After reviewing, I think the idea of having a versioned name-hash is a
good one, and I agree that it'll make the eventual .bitmap changes much
easier to implement.

So I think in that sense exposing a `--name-hash-version` is the right
thing to do. My feeling is that we should probably just add Jonathan's
"v2", since it appears to be a improvement in nearly all cases against
v1, and more often an improvement than not when compared to v3. In that
world, just introducing v2 leaves us with less code to maintain and
fewer, clearer options presented to users.

If you feel strongly about keeping v3, I am definitely open to changing
my mind here, but my feeling on first blush of this most recent round is
that I would probably just include v2.

I'm excited about seeing these patches land, and I am glad that someone
is working on them!

Thanks,
Taylor