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
Retain executor cache counts #22322
Retain executor cache counts #22322
Conversation
Looks like a good incremental improvement to me, just needs tests |
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.
lgtm! Just one suggestion for you to take or leave
runtime/src/bank.rs
Outdated
if count < least { | ||
least = count; | ||
least_key = key; | ||
let entry = if let Some(entry) = self.executors.get(pubkey) { |
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.
If we remove()
instead of get()
here, the entry can be reused and avoid the atomic ops
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.
Yes, much better, I also added a bit more cleanup possible due to your suggestion, thanks!
Codecov Report
@@ Coverage Diff @@
## master #22322 +/- ##
=========================================
- Coverage 81.1% 81.1% -0.1%
=========================================
Files 523 551 +28
Lines 146793 149606 +2813
=========================================
+ Hits 119067 121334 +2267
- Misses 27726 28272 +546 |
(cherry picked from commit f1e2598)
(cherry picked from commit f1e2598)
Problem
When an executor cache entry is re-added its counts are cleared, wiping out its history even though it might be used frequently
Summary of Changes
Retain those counts
Fixes #