runtime: swap im for imbl in StakesCache#10762
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #10762 +/- ##
=======================================
Coverage 83.1% 83.1%
=======================================
Files 858 858
Lines 321771 321771
=======================================
+ Hits 267642 267643 +1
+ Misses 54129 54128 -1 🚀 New features to boost your workflow:
|
|
I'll try to profile this whenever I have time. |
|
similar proportional savings using |
66c9d4d to
caa4d1b
Compare
c992665 to
bb0b6d1
Compare
|
@vadorovsky i landed the necessary solana-sdk work that was blocking this, so we can move forward on this any time we feel happy with it |
92ad741 to
48fade9
Compare
|
Are we going to land this change or did it turn out to not be useful? I'm asking because I want to support wincode serialization of snapshots (which does serialization of structs holding the stakes map) and could focus on adding support for just one of those crates. |
|
im in favor and think its up to @vadorovsky or @alessandrod |
|
@kskalski could you help review? I think switching is ok. The other crate is unmaintained. I did some profiling a few weeks ago and there was maybe a small regression somewhere but overall perf looks good. I say let's go for it and if we find issues we fix them, can't really depend on unmaintained code forever anyway. |
|
I will also compare the performance of into and from iterator that is part of serializing snapshots. |
kskalski
left a comment
There was a problem hiding this comment.
I compared profiles for:
- deserializing snapshot - same perf, though
im*isn't really visible there - serializing snapshot - there is a slight (11% -> 8.5% of cpu), but clear improvement iterating over the im hash map (a lot of the calls into aux code from Iterator::next is gone, so this part was clearly inlined / optimized)
Given benchmark results, other comparison and maintenance status of im I also think it will be good to merge.
Problem
stakes cache is a bottleneck
Summary of Changes
replace
imwithimbl. the former has not been updated for years, the latter is a backwards-compat fork that is presently maintained and more optimizedin #10760 i add some very simple benches for epoch rewards and advancing the epoch. if they are to be believed, this speeds rewards up by 10% and epoch advance by 30%
before:
after: