-
-
Notifications
You must be signed in to change notification settings - Fork 290
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
feat: implement shuffling cache #6030
Conversation
Performance Report✔️ no performance regression detected Full benchmark results
|
2d0d4ff
to
9fdb15b
Compare
29be86f
to
a4b3378
Compare
a4b3378
to
8b86487
Compare
after 4 days testing, metrics are the same to
ready for review @dapplion |
* - skip computing shuffling when loading state bytes from disk | ||
*/ | ||
export class ShufflingCache { | ||
/** LRU cache implemented as an array, pruned every time we add an item */ |
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.
This comment looks wrong, its implemented as a map
*/ | ||
insertPromise(shufflingEpoch: Epoch, decisionRootHex: RootHex): void { | ||
const promiseCount = Array.from(this.itemsByDecisionRootByEpoch.values()) | ||
.map((innerMap) => Array.from(innerMap.values())) |
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.
I think theres a flatMap now
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.
thanks @wemeetagain that looks great 👍
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.
Feature looks correct would like to get this deployed. There are good metrics that should proof it works well
adding metrics here:
|
🎉 This PR is included in v1.13.0 🎉 |
Motivation
Description
this is a prerequisite for #6008
Closes #2848