Make goto_end_of_slot() take Arc<Bank> #33650
Conversation
| for _ in 0..MAX_RECENT_BLOCKHASHES + 1 { | ||
| goto_end_of_slot(Arc::get_mut(&mut bank).unwrap()); | ||
| goto_end_of_slot(bank.clone()); | ||
| bank = Arc::new(new_from_parent(bank)); | ||
| } |
There was a problem hiding this comment.
i know this for is sickeningly copy-pasted below.. but i forgo fixing them all, because i'm lazy. hope i can be regarded as doing my part of cleaning job here. ;)
| }; | ||
| pub fn goto_end_of_slot(bank: &Bank) { | ||
| pub fn goto_end_of_slot(bank: Arc<Bank>) { |
There was a problem hiding this comment.
yes, taking ownership of Arc<Bank> is intentional for upcoming changes. strictly speaking, implied cloning isn't needed, however i leaned towards coding simplicity over perfection.
after all, this isn't part of hot path, rather a tiny and aged test helper function... who cares for additional clones for it? ;)
Codecov Report
@@ Coverage Diff @@
## master #33650 +/- ##
=======================================
Coverage 81.8% 81.8%
=======================================
Files 806 806
Lines 217477 217477
=======================================
+ Hits 177999 178019 +20
+ Misses 39478 39458 -20 |
There was a problem hiding this comment.
Definitely like all the clean up of unneccessary get_muts.
It basically seems in the upcoming changes of #33070, that register_tick (maybe freeze?) has changed impls and within the wrapping BankWithScheduler. This requires passing a scheduler (if one exists), so we will require this function to create a BankWithScheduler internally, which requires ownership.
There's probably some trickery we could do with Cow to get this so we can avoid the additional cloning...but it's probably not worth the effort for avoiding the clone in a fn only used in testing contexts.
Problem
my scheduler thing needs to introduce rather invasive code changes including the
BankWithSchedulerwrapping, which touches the quite long-livedgoto_end_of_slot()test helper function.Summary of Changes
In preparation for the scheduler, adjust its argument type in advance to take
Arc<Bank>, also clean up odd::get_mut()and others for consistent call-site coding styles as a bonus.extracted from: #33070