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.

Sign up for GitHub

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

Jump to bottom

Snapshots cloned during sync #2944

Closed
michaelsproul opened this issue Jan 21, 2022 · 1 comment
Closed

Snapshots cloned during sync #2944

michaelsproul opened this issue Jan 21, 2022 · 1 comment
Labels
optimization Something to make Lighthouse run more efficiently. syncing

Comments

@michaelsproul
Copy link
Member

Description

Since #2832 Lighthouse will clone a snapshot from the cache whenever there's a skipped slot. This is designed to help during live block processing, but also gets applied during sync where it isn't very useful.

Present Behaviour

The condition here trips regardless of whether or not Lighthouse is syncing:

lighthouse/beacon_node/beacon_chain/src/snapshot_cache.rs

Lines 256 to 258 in 79db2d4

if block_slot > cache.beacon_block.slot() + 1 {
return (cache.clone_as_pre_state(), true);
}

Expected Behaviour

Lighthouse should always move the snapshot from the cache during sync in order to prevent unnecessary cloning and extra memory usage.
@michaelsproul michaelsproul added syncing optimization Something to make Lighthouse run more efficiently. labels Jan 21, 2022
@michaelsproul michaelsproul mentioned this issue May 23, 2022
Open
8 tasks
@michaelsproul michaelsproul mentioned this issue Jun 15, 2022
Closed
bors bot pushed a commit that referenced this issue Jun 20, 2022
@michaelsproul
Avoid cloning snapshots during sync (#3271)
efebf71 
## Issue Addressed

Closes #2944

## Proposed Changes

Remove snapshots from the cache during sync rather than cloning them. This reduces unnecessary cloning and memory fragmentation during sync.

## Additional Info

This PR relies on the fact that the `block_delay` cache is not populated for blocks from sync. Relying on block delay may have the side effect that a change in `block_delay` calculation could lead to: a) more clones, if block delays are added for syncing blocks or b) less clones, if blocks near the head are erroneously provided without a `block_delay`. Case (a) would be a regression to the current status quo, and (b) is low-risk given we know that the snapshot cache is current susceptible to misses (hence `tree-states`).
@macladson
Copy link
Member

Closed by #3271

@michaelsproul michaelsproul mentioned this issue Nov 15, 2022
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
optimization Something to make Lighthouse run more efficiently. syncing
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

[Merged by Bors] - Avoid cloning snapshots during sync michaelsproul/lighthouse
2 participants
@michaelsproul @macladson