v1.17: Use BankForks on tests - Part 1 (backport of #34206)
#34354
Conversation
![mergify[bot]](https://avatars.githubusercontent.com/in/10562?s=60&v=4){kind=small}
--------- Signed-off-by: Lucas Steuernagel <[email protected]> (cherry picked from commit e832765) # Conflicts: # runtime/src/bank/tests.rs
|
Hi, @LucasSte. Can you include why this PR should/needs to be backported? |
Fixing these tests are needed to remove |
Why? Can you please explain more? Is there a bug this is fixing? |
The problem manifests as a performance issue during loading of missing programs in the cache. If multiple transaction batches try to load the same program, they all JIT the same program in parallel. That results into unnecessary work, and memory thrashing, leading to other issues. Here's the PR that will fix it, but it's dependent on fixing these tests: #33715 |
| .unwrap() | ||
| .insert(bank) | ||
| .clone_without_scheduler() | ||
| bank_forks.write().unwrap().insert(bank) |
There was a problem hiding this comment.
Insert returns BankWithScheduler, but the function returns Arc<Bank>, so I think we need the clone_without_scheduler here, don't we?
There was a problem hiding this comment.
v1.17 doesn't seem to have that.
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## v1.17 #34354 +/- ##
=========================================
- Coverage 81.8% 81.8% -0.1%
=========================================
Files 803 803
Lines 218106 218152 +46
=========================================
+ Hits 178538 178561 +23
- Misses 39568 39591 +23 |
This is an automatic backport of pull request #34206 done by Mergify.
Cherry-pick of e832765 has failed:
Fixing these tests are needed to remove
WorkingSlotfromLoadedProgramscache. We need to removeWorkingSlotto support cooperative program loading (for cold programs). The cooperative program loading is needed to be backported to v1.17, unless we find a minimal solution that works.