-
Notifications
You must be signed in to change notification settings - Fork 291
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
blockchain: Optimize stake node pruning. #2294
Merged
davecgh
merged 3 commits into
decred:master
from
davecgh:blockchain_optimize_stakenode_prune
Jul 27, 2020
Merged
blockchain: Optimize stake node pruning. #2294
davecgh
merged 3 commits into
decred:master
from
davecgh:blockchain_optimize_stakenode_prune
Jul 27, 2020
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
matheusd
reviewed
Jul 24, 2020
matheusd
approved these changes
Jul 24, 2020
davecgh
force-pushed
the
blockchain_optimize_stakenode_prune
branch
from
July 24, 2020 19:21
8a5d2f4
to
afd0e0c
Compare
dajohi
approved these changes
Jul 24, 2020
davecgh
force-pushed
the
blockchain_optimize_stakenode_prune
branch
2 times, most recently
from
July 24, 2020 19:43
5b68fb8
to
1e5114c
Compare
matheusd
approved these changes
Jul 24, 2020
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.
Thank you for the extended explanation!
davecgh
force-pushed
the
blockchain_optimize_stakenode_prune
branch
2 times, most recently
from
July 25, 2020 07:37
08f1800
to
1263ffb
Compare
This renames the field that tracks the last time the stake nodes were pruned to lastPruneTime to more accurately reflect its purpose.
This modifies the pruning logic to set the pruning interval to the target block time for the given chain parameters instead of hard coding a constant with the mainnet value.
Profiling revealed that the number one cause of allocations in blockchain is currently due to stake node pruning, which definitely should not be the case. Upon closer examination, the culprit is that every single time the stake nodes are pruned the entire ancestry of nodes prior to the prune height, including those that have already been pruned, are appended to the list of nodes to prune. In practice, this amounts to hundreds of thousands of nodes and thus the slice is resized multiple times (likely around 18 times per run given the current runtime semantics). This remedies that by ensuring that only the nodes that actually need to be pruned are added to the relevant slice and also preallocating space per a hint that will handle the number of nodes expected to be pruned with a 99% confidence level to further reduce allocations when more than a single node needs to be pruned. It also renames the slice to reflect what it actually does.
davecgh
force-pushed
the
blockchain_optimize_stakenode_prune
branch
from
July 27, 2020 17:21
1263ffb
to
d7532bd
Compare
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.
Profiling revealed that the number one cause of allocations in blockchain is currently due to stake node pruning, which definitely should not be the case.
Upon closer examination, the culprit is that every single time the stake nodes are pruned the entire ancestry of nodes prior to the prune height, including those that have already been pruned, are appended to the list of nodes to prune. In practice, this amounts to hundreds of thousands of nodes and thus the slice is resized multiple times (likely around 18 times per run given the current runtime semantics).
This remedies that by ensuring that only the nodes that actually need to be pruned are added to the relevant slice and also preallocating space per a hint that will handle the number of nodes expected to be pruned with a 99% confidence level to further reduce allocations when more than a single node needs to be pruned.
It also renames the slice to reflect what it actually does.
Finally, this renames the field used to track the last pruning time to more accurately reflect its purpose
Relevant portion of the before profile:
It does not even show up in the profile with this PR.