rippled
rippled copied to clipboard
Upgrade to xxhash 0.8.2 as conan dependency, enable SIMD hashing
High Level Overview of Change
Upgrade from xxhash from 0.6.2 to 0.8.2 , switch to conan dependency , enable SIMD
Context of Change
We are currently using old version 0.6.2 of xxhash, as a verbatim copy & paste from its header file xxhash.h. Switch to the more recent version 0.8.2 . Since this version is supported by conan (and properly protects its ABI by keeping the state object incomplete), add it as a conan dependency. Also, since this new version supports SIMD instructions (in the new XXH3 family of functions), switch to the new functions to make use of these more performant CPU instructions.
Type of Change
- [ ] Bug fix (non-breaking change which fixes an issue)
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing functionality to not work as expected)
- [ ] Refactor (non-breaking change that only restructures code)
- [x] Performance (increase or change in throughput and/or latency)
- [ ] Tests (you added tests for code that already exists, or your new feature included in this PR)
- [ ] Documentation update
- [ ] Chore (no impact to binary, e.g.
.gitignore, formatting, dropping support for older tooling) - [ ] Release
This change requires performance testing.
Internal tracker: RPFC-85
Codecov Report
Attention: Patch coverage is 85.71429% with 2 lines in your changes are missing coverage. Please review.
Project coverage is 76.94%. Comparing base (
22b7518) to head (6843b1e). Report is 4 commits behind head on develop.
| Files | Patch % | Lines |
|---|---|---|
| src/ripple/beast/hash/xxhasher.h | 85.71% | 1 Missing and 1 partial :warning: |
Additional details and impacted files
@@ Coverage Diff @@
## develop #4893 +/- ##
============================================
+ Coverage 61.65% 76.94% +15.29%
============================================
Files 806 1127 +321
Lines 70967 131676 +60709
Branches 36690 39515 +2825
============================================
+ Hits 43752 101323 +57571
- Misses 19856 24395 +4539
+ Partials 7359 5958 -1401
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
This has cleared performance testing in https://ripplelabs.atlassian.net/browse/RPFC-85
- awaiting review by @ximinez (or suggestion of alternate reviewer)
XXH64_updateinstead ofXXH3_64bits_update
Yes, this the upgrade to SIMD instructions part. In both cases (old and new code) we are operating on 64-bit data type.
Everything looks good. I'm not very familiar with this library, so I took a quick look through the documentation example. I noticed that it does a lot more error checking than this version. Is that something we should be concerned about?
Huh, maybe I need to add handling running out of memory in the constructors, otherwise other operations will silently fail. How do we normally handle it in this project ?
XXH_PUBLIC_API XXH3_state_t* XXH3_createState(void)
{
XXH3_state_t* const state = (XXH3_state_t*)XXH_alignedMalloc(sizeof(XXH3_state_t), 64);
if (state==NULL) return NULL;
No, you don't need to do anything special about running out of memory. If rippled runs out of memory it bursts into flame. There's no special handling for low memory situations.
No, you don't need to do anything special about running out of memory. If rippled runs out of memory it bursts into flame. There's no special handling for low memory situations.
In this case throwing std::bad_alloc is better than failing silently. I will add it.
@ximinez do you mind checking again ? I added throw std::bad_alloc() where appropriate. There are no other error paths that I can see in in xxhash sources.