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

Clone state ahead of block production #4925

Merged
merged 3 commits into from
Nov 30, 2023

Conversation

michaelsproul
Copy link
Member

Proposed Changes

When proposing, clone the state for the proposal 500ms before the start of the slot and cache it. We then move this clone into position when the request to build a block comes in.

This should shave 250-500ms off block production on larger networks like mainnet and Holesky. We won't need this when we have tree-states, but this is a relatively simple patch to keep us competitive until then.

@michaelsproul michaelsproul added optimization Something to make Lighthouse run more efficiently. v4.6.0 ETA Q1 2024 labels Nov 15, 2023
@michaelsproul
Copy link
Member Author

Shaved 500ms off block production on Holesky:

holesky2

holesky

@michaelsproul michaelsproul added the ready-for-review The code is ready for review label Nov 19, 2023
@paulhauner paulhauner self-requested a review November 20, 2023 00:10
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 optimisation! I just have a question about pruning.

beacon_node/beacon_chain/src/state_advance_timer.rs Outdated Show resolved Hide resolved
@paulhauner paulhauner self-requested a review November 21, 2023 04:26
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.

Looks good, I just have a suggestion about avoiding holding two states in memory. I'm happy with the PR as-is though.

// If we're proposing, clone the head state preemptively so that it isn't on
// the hot path of proposing. We can delete this once we have tree-states.
if let Some(proposer_head) = proposer_head {
if let Some(proposer_state) = beacon_chain
Copy link
Member

@paulhauner paulhauner Nov 21, 2023

Choose a reason for hiding this comment

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

I wonder if it would be beneficial to drop the block_production_state above this line. It would prevent us from holding the old state in memory whilst we load the new one.

The downside would be that there are some times where we clear the block_production_state when we don't need to (i.e. the state is not in the snapshot cache but would be useful for the next proposal).

Copy link
Member Author

Choose a reason for hiding this comment

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

Good idea. I've added that in 6d5d558. It will still hold 2 states in memory if the beacon node has two proposals two slots in a row on the same head block, but I think this is reasonably obscure enough to be tolerated until we get tree-states. I'll try to deploy that change on Holesky in a few hours so I can confirm it didn't regress on the perf front.

@michaelsproul
Copy link
Member Author

I think this is good to merge. No noticeable memory spikes on the Holesky node where it's running, and block production times continue to show a ~500ms reduction.

OK to merge @paulhauner ?

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.

LGTM! Merge at will!

@michaelsproul michaelsproul merged commit 547ed1d into sigp:unstable Nov 30, 2023
24 checks passed
@michaelsproul michaelsproul deleted the cache-block-production branch November 30, 2023 02:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
optimization Something to make Lighthouse run more efficiently. ready-for-review The code is ready for review v4.6.0 ETA Q1 2024
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants