-
Notifications
You must be signed in to change notification settings - Fork 4.4k
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
additional costs in block capacity calc #25059
Merged
jdavis103
merged 7 commits into
solana-labs:master
from
jdavis103:jason-additional-costs-in-block-capacity-calc
May 12, 2022
Merged
additional costs in block capacity calc #25059
jdavis103
merged 7 commits into
solana-labs:master
from
jdavis103:jason-additional-costs-in-block-capacity-calc
May 12, 2022
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…c of CostModel all the way to the top of the call chain, instead of reallocing
tao-stones
reviewed
May 6, 2022
Codecov Report
@@ Coverage Diff @@
## master #25059 +/- ##
=========================================
Coverage 82.0% 82.0%
=========================================
Files 598 610 +12
Lines 165882 167963 +2081
=========================================
+ Hits 136125 137893 +1768
- Misses 29757 30070 +313 |
tao-stones
changed the title
Jason additional costs in block capacity calc
additional costs in block capacity calc
May 11, 2022
tao-stones
reviewed
May 11, 2022
tao-stones
reviewed
May 11, 2022
tao-stones
approved these changes
May 12, 2022
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, probably should complete issue #25166 before enabling featureset
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Problem
A leader could put more transactions into a block than can be completed during the slot. Could put validators behind on future blocks, an even bigger issue if that validator will soon be a leader.
Summary of Changes
If, during replay, we've already taken more time (in compute-unit terms) than fits in a slot, existing code stops work on this block and moves on (if this feature is turned on). This PR expands the capacity-gating of blocks during replay to not only include execution time used, but also three static values -- signature, write-lock, and data bytes costs. This will give a more accurate (and earlier) stopping point if needed.
There is also a change to pass around a single CostModel structure throughout this file, as that is a comparatively expensive structure to initialize, and we need it in the new cost-estimating function.
This PR is a re-issue of the code from #24664, which is now streamlined due to some other cleanups around it.
Fixes #