Skip to content

Prefer XXHash instead of CRC for IP10 #2500

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
wants to merge 8 commits into
base: main
Choose a base branch
from
Open

Prefer XXHash instead of CRC for IP10 #2500

wants to merge 8 commits into from

Conversation

t-b
Copy link
Collaborator

@t-b t-b commented Sep 3, 2025 •
edited
Loading

  • Resolve FIXMEs
  • add check for no StringCRC/WaveCRC function calls with NOLINT support
  • Raise minimum IP10 version (build: 29663)
  • Rework cache generator functions to have the version always at the end, include plaintext identifier
  • Move HashNumber/HashString/HashWave to utils
  • Port use of SHAXXX where appropriate to new Hash functions

Close #2162

@t-b t-b self-assigned this Sep 3, 2025
t-b added 8 commits September 5, 2025 19:08
@t-b
Add hash functions for strings, numbers and waves
ef77c84 
These uses XXH3-64 for IP10 and CRC for IP9.
@t-b
AB_LoadSweepFromNWBgeneric: Prefer HashWave over WaveCRC
c31f977 
This is safer on and faster on IP10, and the uses CRC as well on IP9.
@t-b
WB_GetStimsetChecksum: Prefer WB_GetWaveNoteEntryAsNumber
7fb5654
@t-b
WB_CalculateStimsetChecksum: Switch from CRC to WaveHash
21ba0d0 
This uses XXH3-64 on IP10 which is **much** better. The repercussions are
not small though. As our general rule that we rename things when the
content changes we now have "Checksum V2" in the stimset note and "Stim
Wave Checksum V2"/"TTL Stim Wave Checksum V2" for the labnotebook. As the
checksum is also not a 32-bit number anymore, this is now also moved into
textual labnotebook.
@t-b
tools/check-code.sh: Check for StringCRC/WaveCRC
fefd456 
We want to use HashString/HashNumber/HashWave in most casesm instead. For
the remaining ones we have NOLINT support.
@t-b
GetNotebookCRC: Prefer HashString and rename
a0f2acf
@t-b
DC_GenerateStimsetFingerprint: Move from StringCRC to HashString
f2c14d4 
And add a textual helper wave for data acquisition to store the hash.
@t-b
WIP: Cache
636da49
@t-b t-b force-pushed the feature/2500-move-to-xxhash-for-ip10 branch from 0a9cee6 to 636da49 Compare September 5, 2025 19:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@timjarsky timjarsky Awaiting requested review from timjarsky timjarsky is a code owner

@MichaelHuth MichaelHuth Awaiting requested review from MichaelHuth MichaelHuth is a code owner

At least 1 approving review is required to merge this pull request.

Assignees

@t-b t-b

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Use better hashing in cache key calcuation

1 participant

@t-b