Re: [PATCH] fix: platform accordance while calculating murmur3

From: Taylor Blau <me@ttaylorr.com>
Date: Jan 4, 2024, 14:53 UTC
Message-ID: <ZZbGZYhhWpHh2pS/@nand.local>
In-Reply-To: <pull.1636.git.git.1704376606625.gitgitgadget@gmail.com>

Hi Chen,

On Thu, Jan 04, 2024 at 01:56:46PM +0000, Chen Xuewei via GitGitGadget wrote:

Show 5 quoted lines

> From: Chen Xuewei <316403398@qq.com>
>
> It is known that whether the highest bit is extended when char cast to
> uint32, depends on CPU architecture, which will lead different hash
> value. This is a fix to accord all architecture behaviour.

Thanks for your patch. A similar fix is being pursued in [1], part of which includes [2], which I believe is functionally equivalent to your patch here.

Show 7 quoted lines

>     Others
>     ======
>
>     after fixed the bug, the historical bloom_filter data stored in
>     commit-graph need to be updated. because the path's hash value is
>     already calculated through a bad way. so we need to update it. this need
>     to be done in repository

We would not want to impose that burden on all users upon upgrading to the latest Git version. In [1] we are perusing an approach where:

  - The Bloom data is stored with a version identifier, meaning that we
    can still use the existing/non-murmur3 Bloom filters after
    upgrading.

  - When the user decides to upgrade from v1 -> v2 Bloom filters, we
    reuse the existing Bloom filter data when possible, namely when all
    paths within a tree have no non-ASCII characters.

If you have thoughts on the approach in [1], they would be most welcome.

Thanks, Taylor

[1]: https://lore.kernel.org/git/cover.1697653929.git.me@ttaylorr.com/ [2]: https://lore.kernel.org/git/f6ab427ead86bc82284b2c721f3c177947ece3c9.1697653929.git.me@ttaylorr.com/