v3.1: accounts-db: Use SmallRng for LRU eviction sampling (backport of #10640)#10644
v3.1: accounts-db: Use SmallRng for LRU eviction sampling (backport of #10640)#10644
SmallRng for LRU eviction sampling (backport of #10640)#10644Conversation
Sampling is the biggest bottleneck in the eviction mechanism, which with the default `Rng` takes 82% of the whole eviction process and causes the overall eviction process to take over 20s. Optimize it by using `SmallRng`. (cherry picked from commit 1ffcdcc) # Conflicts: # accounts-db/src/read_only_accounts_cache.rs
|
Cherry-pick of 1ffcdcc has failed: To fix up this pull request, you can check it out locally. See documentation: https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/checking-out-pull-requests-locally |
|
@vadorovsky mind resolving merge conflicts here? |
bb4800e to
645b8f3
Compare
Done. To elaborate on what I had to fix, the ctor for an RNG seeded from In In I don't want that info to get lost - is it fine if after approval, before merging the PR, I squash the commits and add the info on the bottom of the commit message? |
The constructor for an RNG seeded from `getrandom` has different names in different `rand_core` versions. In `rand_core-0.6.4` it's called SeedableRng::from_entropy`.[0][1] In `rand_core-0.9.3` it's called `SeedableRng::from_os_rng`.[2][3] Furthermore, `SmallRng` in `rand-0.8.5` is hidden behind `small_rng` feature. [0] https://docs.rs/rand_core/0.6.4/rand_core/trait.SeedableRng.html#method.from_entropy [1] https://github.com/rust-random/rand_core/blob/89a1336b934c68ddce548127c6f8afd910b35a18/rand_core/src/lib.rs#L410-L418Th [2] https://docs.rs/rand_core/0.9.3/rand_core/trait.SeedableRng.html#method.from_os_rng [3] https://github.com/rust-random/rand_core/blob/2d84d25c45a68c1424e4fa775d8a527dc16cec65/src/lib.rs#L554-L577
645b8f3 to
cc69c00
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## v3.1 #10644 +/- ##
=========================================
- Coverage 83.2% 83.2% -0.1%
=========================================
Files 865 865
Lines 376601 376601
=========================================
- Hits 313609 313550 -59
- Misses 62992 63051 +59 🚀 New features to boost your workflow:
|
I'm going to merge this but will ensure this information is retained; looks like your additional commit had good information that will leave us a nice papertrail |
Problem
Sampling is the biggest bottleneck in the eviction mechanism, which with the default
Rngtakes 82% of the whole eviction process and causes the overall eviction process to take over 20s.Summary of Changes
Optimize it by using
SmallRng.☝️ The new profiles are still not ideal - the lock contention, previously taking 11% of time, now is the biggest bottleneck taking 91%. But that's going away in #10641.
This is an automatic backport of pull request #10640 done by Mergify.