From efebf712dd2ff272e75dd3d59c56a8219d34a5a1 Mon Sep 17 00:00:00 2001 From: Michael Sproul Date: Mon, 20 Jun 2022 23:20:29 +0000 Subject: [PATCH] Avoid cloning snapshots during sync (#3271) ## Issue Addressed Closes https://github.com/sigp/lighthouse/issues/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`). --- beacon_node/beacon_chain/src/snapshot_cache.rs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/beacon_node/beacon_chain/src/snapshot_cache.rs b/beacon_node/beacon_chain/src/snapshot_cache.rs index 5585581362..d5b41366cc 100644 --- a/beacon_node/beacon_chain/src/snapshot_cache.rs +++ b/beacon_node/beacon_chain/src/snapshot_cache.rs @@ -253,12 +253,11 @@ impl SnapshotCache { .position(|snapshot| snapshot.beacon_block_root == block_root) .map(|i| { if let Some(cache) = self.snapshots.get(i) { - if block_slot > cache.beacon_block.slot() + 1 { - return (cache.clone_as_pre_state(), true); - } + // Avoid cloning the block during sync (when the `block_delay` is `None`). if let Some(delay) = block_delay { if delay >= MINIMUM_BLOCK_DELAY_FOR_CLONE && delay <= Duration::from_secs(spec.seconds_per_slot) * 4 + || block_slot > cache.beacon_block.slot() + 1 { return (cache.clone_as_pre_state(), true); }