-
Notifications
You must be signed in to change notification settings - Fork 587
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.
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
[Memtrie] Optimize NibbleSlice during the memtrie construction code path #11342
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #11342 +/- ##
==========================================
+ Coverage 71.08% 71.09% +0.01%
==========================================
Files 783 784 +1
Lines 156875 156945 +70
Branches 156875 156945 +70
==========================================
+ Hits 111508 111580 +72
- Misses 40528 40539 +11
+ Partials 4839 4826 -13
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I did not get a detailed view of the nibble_slice.rs but the other changes LGTM. (I assume nibble_slice.rs is well tested anyways).
use super::node::MemTrieNodeId; | ||
use crate::trie::mem::node::InputMemTrieNode; | ||
use crate::NibbleSlice; | ||
use elastic_array::ElasticArray4; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks like an iffy crate to use compared to the much better established smallvec
: the documentation is pretty light, the maintenance status is unclear and its just plain a less popular crate.
Though I guess this is a preexisting dependency, so I'll file an issue to address this instead...
…ath (near#11342) This contains two top optimizations that speed up the memtrie construction code: * Rather than allocating a new Vec<u8> every time we encode nibbles, we use a freelist so that previously allocated Vec<u8> can be reused. This reduces allocations from O(millions) to O(tens). * As a side effect, InputMemTrieNode has to be changed to contain borrowed slices, as we can no longer give owned Box<[u8]> to it. This is totally fine though because anyway the InputMemTrieNode's data needs to be copied into the constructed memtrie nodes in the arena. * Optimize the nibble slice encoding function so that rather than encoding nibble by nibble, we encode the first nibble and then just copy the remaining bytes directly. Also optimize the at(i) function, removing branching entirely (this code now gets compiled to a bunch of arithmetic operations). On shard 5 these optimizations reduce load time from 146s to 122s.
This contains two top optimizations that speed up the memtrie construction code:
On shard 5 these optimizations reduce load time from 146s to 122s.