Skip to content
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] - Optimize process_attestation with active balance cache #2560

Closed
wants to merge 3 commits into from

Conversation

michaelsproul
Copy link
Member

@michaelsproul michaelsproul commented Sep 3, 2021

Proposed Changes

Cache the total active balance for the current epoch in the BeaconState. Computing this value takes around 1ms, and this was negatively impacting block processing times on Prater, particularly when reconstructing states.

With a large number of attestations in each block, I saw the process_attestations function taking 150ms, which means that reconstructing hot states can take up to 4.65s (31 * 150ms), and reconstructing freezer states can take up to 307s (2047 * 150ms).

I opted to add the cache to the beacon state rather than computing the total active balance at the start of state processing and threading it through. Although this would be simpler in a way, it would waste time, particularly during block replay, as the total active balance doesn't change for the duration of an epoch. So we save ~32ms for hot states, and up to 8.1s for freezer states (using --slots-per-restore-point 8192).

@michaelsproul michaelsproul added A0 bug Something isn't working optimization Something to make Lighthouse run more efficiently. ready-for-review The code is ready for review labels Sep 3, 2021
@michaelsproul
Copy link
Member Author

Testing on a Prater node with --slots-per-restore-point 32 I measured these times:

  • ~550ms to load a historic state with this PR, vs 4.6s without
  • 500ms to compute the response for the validator inclusion API, vs 5s+ without

Copy link
Member

@paulhauner paulhauner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great fix!

@michaelsproul michaelsproul added ready-for-merge This PR is ready to merge. and removed ready-for-review The code is ready for review labels Sep 3, 2021
@michaelsproul
Copy link
Member Author

Thank you!

bors r+

bors bot pushed a commit that referenced this pull request Sep 3, 2021
## Proposed Changes

Cache the total active balance for the current epoch in the `BeaconState`. Computing this value takes around 1ms, and this was negatively impacting block processing times on Prater, particularly when reconstructing states.

With a large number of attestations in each block, I saw the `process_attestations` function taking 150ms, which means that reconstructing hot states can take up to 4.65s (31 * 150ms), and reconstructing freezer states can take up to 307s (2047 * 150ms).

I opted to add the cache to the beacon state rather than computing the total active balance at the start of state processing and threading it through. Although this would be simpler in a way, it would waste time, particularly during block replay, as the total active balance doesn't change for the duration of an epoch. So we save ~32ms for hot states, and up to 8.1s for freezer states (using `--slots-per-restore-point 8192`).
@bors bors bot changed the title Optimize process_attestation with active balance cache [Merged by Bors] - Optimize process_attestation with active balance cache Sep 3, 2021
@bors bors bot closed this Sep 3, 2021
@michaelsproul michaelsproul deleted the active-balance-cache branch September 3, 2021 10:40
@guybrush
Copy link

guybrush commented Sep 3, 2021

thank you!

right now https://prater.beaconcha.in is behind because this endpoint is slow (note: it works just fine on pyrmont).

also some epochs will be shown with 0 eligible ether and participation-rate, we export epochs with 0 values on timeout right now. will fix these epochs after deployed fix.

@michaelsproul michaelsproul mentioned this pull request Sep 20, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A0 bug Something isn't working optimization Something to make Lighthouse run more efficiently. ready-for-merge This PR is ready to merge.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants