From 2652542ee7457def50f2ab639273371d91a41d93 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 15 Jul 2026 11:03:16 +0000 Subject: [PATCH 1/2] starknet_patricia: actually pre-allocate filled-tree output map capacity PR #14638 intended to pre-allocate initialize_filled_tree_output_map_with_placeholders's HashMap to avoid O(log N) rehashing while filling the tree (once per block, per tree), but the filter_map().collect() it introduced doesn't achieve this: FilterMap::size_hint() always reports a lower bound of 0, and HashMap's Extend impl reserves capacity from that lower bound, so no pre-allocation happens. Reserve capacity from the raw node iterator (exact size_hint, since it comes from a HashMap::iter()) before filtering instead. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_015cTjgteomFa2zu4xh6L2yJ --- .../src/patricia_merkle_tree/filled_tree/tree.rs | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/crates/starknet_patricia/src/patricia_merkle_tree/filled_tree/tree.rs b/crates/starknet_patricia/src/patricia_merkle_tree/filled_tree/tree.rs index 31f07034621..26cf67c35fc 100644 --- a/crates/starknet_patricia/src/patricia_merkle_tree/filled_tree/tree.rs +++ b/crates/starknet_patricia/src/patricia_merkle_tree/filled_tree/tree.rs @@ -77,13 +77,15 @@ impl FilledTreeImpl { fn initialize_filled_tree_output_map_with_placeholders<'a>( updated_skeleton: &impl UpdatedSkeletonTree<'a>, ) -> HashMap>> { - updated_skeleton - .get_nodes() - .filter_map(|(index, node)| match node { - UpdatedSkeletonNode::UnmodifiedSubTree(_) => None, - _ => Some((*index, OnceLock::new())), - }) - .collect() + // `filter_map` erases the exact lower bound of `size_hint`, so capacity must be reserved + // from the underlying node iterator before filtering to actually avoid rehashing. + let nodes = updated_skeleton.get_nodes(); + let mut filled_tree_output_map = HashMap::with_capacity(nodes.size_hint().0); + filled_tree_output_map.extend(nodes.filter_map(|(index, node)| match node { + UpdatedSkeletonNode::UnmodifiedSubTree(_) => None, + _ => Some((*index, OnceLock::new())), + })); + filled_tree_output_map } fn initialize_leaf_output_map_with_placeholders( From 1eab7807b3fd63ef90b551c1ee73bd88d6be390c Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 15 Jul 2026 11:08:15 +0000 Subject: [PATCH 2/2] starknet_patricia: document size_hint contract and over-allocation tradeoff Address Opus review feedback on the prior commit: document on the get_nodes trait method that callers rely on an exact size_hint (so future implementations don't silently regress the pre-allocation), note the deliberate over-allocation for filtered UnmodifiedSubTree nodes, and rename the local iterator binding for clarity. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_015cTjgteomFa2zu4xh6L2yJ --- .../src/patricia_merkle_tree/filled_tree/tree.rs | 11 +++++++---- .../updated_skeleton_tree/tree.rs | 4 +++- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/crates/starknet_patricia/src/patricia_merkle_tree/filled_tree/tree.rs b/crates/starknet_patricia/src/patricia_merkle_tree/filled_tree/tree.rs index 26cf67c35fc..8b2ef008e88 100644 --- a/crates/starknet_patricia/src/patricia_merkle_tree/filled_tree/tree.rs +++ b/crates/starknet_patricia/src/patricia_merkle_tree/filled_tree/tree.rs @@ -78,10 +78,13 @@ impl FilledTreeImpl { updated_skeleton: &impl UpdatedSkeletonTree<'a>, ) -> HashMap>> { // `filter_map` erases the exact lower bound of `size_hint`, so capacity must be reserved - // from the underlying node iterator before filtering to actually avoid rehashing. - let nodes = updated_skeleton.get_nodes(); - let mut filled_tree_output_map = HashMap::with_capacity(nodes.size_hint().0); - filled_tree_output_map.extend(nodes.filter_map(|(index, node)| match node { + // from the underlying node iterator (whose `size_hint` is exact, since it comes from a + // `HashMap::iter()`) before filtering, to actually avoid rehashing. This over-reserves for + // the `UnmodifiedSubTree` nodes filtered out below, trading bounded extra capacity for + // skipping a separate counting pass. + let node_iterator = updated_skeleton.get_nodes(); + let mut filled_tree_output_map = HashMap::with_capacity(node_iterator.size_hint().0); + filled_tree_output_map.extend(node_iterator.filter_map(|(index, node)| match node { UpdatedSkeletonNode::UnmodifiedSubTree(_) => None, _ => Some((*index, OnceLock::new())), })); diff --git a/crates/starknet_patricia/src/patricia_merkle_tree/updated_skeleton_tree/tree.rs b/crates/starknet_patricia/src/patricia_merkle_tree/updated_skeleton_tree/tree.rs index 0ca5a28957d..11a739a589f 100644 --- a/crates/starknet_patricia/src/patricia_merkle_tree/updated_skeleton_tree/tree.rs +++ b/crates/starknet_patricia/src/patricia_merkle_tree/updated_skeleton_tree/tree.rs @@ -30,7 +30,9 @@ pub trait UpdatedSkeletonTree<'a>: Sized + Send + Sync { /// Does the skeleton represents an empty-tree (i.e. all leaves are empty). fn is_empty(&self) -> bool; - /// Returns an iterator over all (node index, node) pairs in the tree. + /// Returns an iterator over all (node index, node) pairs in the tree. Callers rely on an + /// exact `size_hint` (lower bound equal to the true count) to pre-allocate; implementations + /// must not wrap this in an adapter (e.g. `filter`) that only yields a lower bound of `0`. fn get_nodes(&self) -> impl Iterator; /// Returns the node with the given index.