Add limit to looping in banking-stage #35342
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #35342 +/- ##
=======================================
Coverage 81.7% 81.7%
=======================================
Files 834 834
Lines 224299 224309 +10
=======================================
+ Hits 183361 183379 +18
+ Misses 40938 40930 -8 |
|
@Lichtso, @alessandrod , could you review the changes in the |
| .finish_cooperative_loading_task(self.slot, key, program) | ||
| && limit_to_load_programs | ||
| { | ||
| let mut ret = LoadedProgramsForTxBatch::default(); |
There was a problem hiding this comment.
We should return the existing loaded_programs_for_txs here instead of a Default one. Otherwise it is not initialized with a slot and environment.
There was a problem hiding this comment.
I copied this from here: da2078d#diff-ed47b4a0198313377e091bb3957bbbc63d937805426d1b2b6de39d0a50d32a0cR5103
Do you think it's a problem, that should be fixed there as well?
There was a problem hiding this comment.
Also, existing loaded_programs_for_txs might have some programs that could've gotten loaded in previous iterations of the loop. So it's not guaranteed to be empty. Maybe we should create a new instance with the correct slot and environment, instead of using default.
There was a problem hiding this comment.
It would be cleaner to return the existing one in case something later reads these fields. Can be done in a separate PR if this shall stay a clean forward port.
There was a problem hiding this comment.
Separate PR would be better. Also, maybe setting the correct slot and environment is better than using the existing instance (https://github.com/solana-labs/solana/pull/35342/files#r1506511480).
Co-authored-by: Trent Nelson <490004+t-nelson@users.noreply.github.com>
|
Thanks for the review. Just committed @t-nelson nit suggestion. |
|
Backports to the beta branch are to be avoided unless absolutely necessary for fixing bugs, security issues, and perf regressions. Changes intended for backport should be structured such that a minimum effective diff can be committed separately from any refactoring, plumbing, cleanup, etc that are not strictly necessary to achieve the goal. Any of the latter should go only into master and ride the normal stabilization schedule. Exceptions include CI/metrics changes, CLI improvements and documentation updates on a case by case basis. |
(cherry picked from commit 990ca1d) # Conflicts: # runtime/src/bank.rs # svm/src/transaction_processor.rs
(cherry picked from commit 990ca1d)
Problem
Forward port of da2078d is missing on master branch.
Summary of Changes
The code has diverged. So manually imported the commit to master branch.
Fixes #