Skip to content

Conversation

TimWolla
Copy link
Member

@TimWolla TimWolla commented Oct 4, 2025

No description provided.

@TimWolla TimWolla changed the title hash: Upgrade xxHash to 0.8.3 hash: Upgrade xxHash to 0.8.2 Oct 4, 2025
@TimWolla
Copy link
Member Author

TimWolla commented Oct 4, 2025

0.8.3 changes the internal struct, which breaks serialization, because serialization touches the internal fields of the opaque struct …

@nielsdos
Copy link
Member

nielsdos commented Oct 5, 2025

I'll have a proper look later, but the xxhash files had some manual patches done to them by various people (mostly backport probably); see #18842

@TimWolla
Copy link
Member Author

TimWolla commented Oct 6, 2025

but the xxhash files had some manual patches done to them

If it's not asserted by CI it doesn't exist 😉

@nielsdos
Copy link
Member

nielsdos commented Oct 6, 2025

Then half of php does not exist

Copy link
Member

@nielsdos nielsdos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I went through the patches that went in after xxhash 0.8.1 went in:

  • a839230 which was pulled from upstream, so OK included
  • 7638640 also from upstream, so OK included
  • adcb38b also upstream
  • a82d864 which also has a corresponding upstream PR
  • afb1c57 which is mine, part of this modifies xxhash.h to work around a (older) compiler bug. I think it's fine to drop this patch.

I then compared with 0.8.2 upstream and found the sources match.

0.8.3 changes the internal struct, which breaks serialization, because serialization touches the internal fields of the opaque struct …

I see. That's unfortunate. I haven't looked at this yet but I wonder if there's a migration path possible. 0.8.3 contains a bugfix for XXH3_128bits_withSecretandSeed but it doesn't seem like we use that directly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants