From: Patrick Steinhardt <ps@pks.im>
Date: Tue, 26 Nov 2024 08:26:30 GMT
Subject: Re: [PATCH 1/7] pack-objects: add --full-name-hash option
Message-ID: <Z0WGLeI2TA8m6Gpu@pks.im>
In-Reply-To: <812257e197cfe30bd0d3c68ea6ec0d062631185f.1730775907.git.gitgitgadget@gmail.com>

On Tue, Nov 05, 2024 at 03:05:01AM +0000, Derrick Stolee via GitGitGadget wrote:
> It is important to point out that the name hash value is stored in the
> .bitmap file format, so we must disable the --full-name-hash option when
> bitmaps are being read or written. Later, the bitmap format could be
> updated to be aware of the name hash version so deltas can be quickly
> computed across the bitmapped/not-bitmapped boundary.

I was wondering a bit about this: is there any reason why we cannot have
both, that is reap the benefits of "--full-name-hash" but end up writing
a bitmap with the old name hash so that we can continue to generate
bitmaps?

Forgive me if this question is naive, I'm more at home in the refs
subsystem :)

> diff --git a/pack-objects.h b/pack-objects.h
> index b9898a4e64b..88360aa3e8e 100644
> --- a/pack-objects.h
> +++ b/pack-objects.h
> @@ -207,6 +207,27 @@ static inline uint32_t pack_name_hash(const char *name)
>  	return hash;
>  }
>  
> +static inline uint32_t pack_full_name_hash(const char *name)
> +{
> +	const uint32_t bigp = 1234572167U;
> +	uint32_t c, hash = bigp;

It would be nice to have a comment here detailing how you came up with
that number, and what its requirements are. You briefly mention it in
the comment further down, but I think this could be expanded a bit.

Patrick