Skip to content
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

Rewrite XXH3_update #805

Merged
merged 2 commits into from
Feb 28, 2023
Merged

Rewrite XXH3_update #805

merged 2 commits into from
Feb 28, 2023

Conversation

easyaspi314
Copy link
Contributor

  • XXH3_consumeStripes can now process multiple stripes, greatly simplifying the logic
  • Tail merged the last XXH3_accumulate_512 in XXH3_digestLong
  • XXH3_128bits_update now just tail calls XXH3_64bits_update to reduce code size

@easyaspi314
Rewrite XXH3_update
44bb94e 
 - XXH3_consumeStripes can now process multiple stripes, greatly simplifying the logic
 - Tail merged the last XXH3_accumulate_512 in XXH3_digestLong
 - XXH3_128bits_update now just tail calls XXH3_64bits_update to reduce code size
Cyan4973
Cyan4973 reviewed Feb 28, 2023
View reviewed changes
xxhash.h Show resolved Hide resolved
@Cyan4973
Copy link
Owner

So, the expected observable impacts of this PR are :

  • Shorter code
  • Smaller produced binaries
  • No speed difference

Cyan4973
Cyan4973 approved these changes Feb 28, 2023
View reviewed changes
Copy link
Owner

@Cyan4973 Cyan4973 left a comment

Choose a reason for hiding this comment

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

There's only one minor code documentation comment,
then it's good to go.

@Cyan4973 Cyan4973 merged commit 2b328a1 into Cyan4973:dev Feb 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Ilqar555 Ilqar555 Ilqar555 approved these changes

@Cyan4973 Cyan4973 Cyan4973 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@easyaspi314 @Cyan4973 @Ilqar555