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
[Merged by Bors] - Optimise balances cache in case of skipped slots #2849
Conversation
I've implemented a schema migration for this change to address the issue described here: https://hackmd.io/@sproul/HyUjTaSFY The migration is here: michaelsproul@f1496b9 I'll push it to this branch once #2822 is merged |
0df386b
to
efd5184
Compare
efd5184
to
549e5b9
Compare
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.
Very nice! Thanks for fixing this, these are my bugs from before I had a grip on caching things like this 🙏
The cache works much nicer now!
549e5b9
to
ec94984
Compare
Going to try an optimistic merge! bors r+ |
## Proposed Changes Remove the `is_first_block_in_epoch` logic from the balances cache update logic, as it was incorrect in the case of skipped slots. The updated code is simpler because regardless of whether the block is the first in the epoch we can check if an entry for the epoch boundary root already exists in the cache, and update the cache accordingly. Additionally, to assist with flip-flopping justified epochs, move to cloning the balance cache rather than moving it. This should still be very fast in practice because the balances cache is a ~1.6MB `Vec`, and this operation is expected to only occur infrequently.
Pull request successfully merged into unstable. Build succeeded: |
Proposed Changes
Remove the
is_first_block_in_epoch
logic from the balances cache update logic, as it was incorrect in the case of skipped slots. The updated code is simpler because regardless of whether the block is the first in the epoch we can check if an entry for the epoch boundary root already exists in the cache, and update the cache accordingly.Additionally, to assist with flip-flopping justified epochs, move to cloning the balance cache rather than moving it. This should still be very fast in practice because the balances cache is a ~1.6MB
Vec
, and this operation is expected to only occur infrequently.