-
Notifications
You must be signed in to change notification settings - Fork 549
Feature/Configurable dirty node shard count #8410
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
Conversation
if (_shardBit is <= 0 or > 30) | ||
{ | ||
throw new InvalidOperationException($"Shard bit count must be between 0 and 30."); | ||
} |
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.
Move this validation directly to config class?
: BinaryPrimitives.ReadUInt32BigEndian(hash.Bytes[..4]); | ||
|
||
// Using some early bits so that nodes of similar prefix would get put in the same shard. | ||
uint shardIdx = baseSize >> (32 - _shardBit); |
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.
32 - _shardBit
could be saved in a field.
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.
Change to use sampling from GetHashCode() so similar paths don't sample together
Co-authored-by: Ben {chmark} Adams <thundercat@illyriad.co.uk>
@@ -79,4 +79,7 @@ The max number of parallel tasks that can be used by full pruning. | |||
|
|||
[ConfigItem(Description = "The number of past states before the state gets pruned. Used to determine how old of a state to keep from the head.", DefaultValue = "64")] | |||
int PruningBoundary { get; set; } | |||
|
|||
[ConfigItem(Description = "Dirty node shard count", DefaultValue = "8")] | |||
int DirtyNodeShardBit { get; set; } |
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.
Should it default to something based on CPU count maybe?
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.
Maybe, one day. But for now I can't find any difference unless it is set too low where pruning can't parallelize.
Co-authored-by: Ben {chmark} Adams <thundercat@illyriad.co.uk>
Co-authored-by: Ben {chmark} Adams <thundercat@illyriad.co.uk>
Looks like failure is persistent
|
Changes
Types of changes
What types of changes does your code introduce?
Testing
Requires testing
If yes, did you write tests?
Notes on testing