From 48e0ec67d00c158ea8376d5454e6b6c0d2e8d319 Mon Sep 17 00:00:00 2001 From: Alexey Shekhirin <5773434+shekhirin@users.noreply.github.com> Date: Thu, 3 Apr 2025 12:54:04 +0100 Subject: [PATCH] perf(trie): cache last hashed entry seek in trie node iter (#15471) --- crates/trie/trie/src/metrics.rs | 4 +- crates/trie/trie/src/node_iter.rs | 70 +++++++++++++++++++++---------- 2 files changed, 50 insertions(+), 24 deletions(-) diff --git a/crates/trie/trie/src/metrics.rs b/crates/trie/trie/src/metrics.rs index 0c8f3762b8..e7c7d4010d 100644 --- a/crates/trie/trie/src/metrics.rs +++ b/crates/trie/trie/src/metrics.rs @@ -72,7 +72,9 @@ impl WalkerMetrics { pub struct TrieNodeIterMetrics { /// The number of branch nodes returned by the iterator. branch_nodes_returned_total: Counter, - /// The number of times same leaf node was seeked multiple times in a row by the iterator. + /// The number of times the same hashed cursor key was seeked multiple times in a row by the + /// iterator. It does not mean mean the database seek was actually done, as the trie node + /// iterator caches the last hashed cursor seek. leaf_nodes_same_seeked_total: Counter, /// The number of times the same leaf node as we just advanced to was seeked by the iterator. leaf_nodes_same_seeked_as_advanced_total: Counter, diff --git a/crates/trie/trie/src/node_iter.rs b/crates/trie/trie/src/node_iter.rs index 215cb943cb..c838edc971 100644 --- a/crates/trie/trie/src/node_iter.rs +++ b/crates/trie/trie/src/node_iter.rs @@ -33,6 +33,17 @@ pub enum TrieElement { Leaf(B256, Value), } +/// Result of calling [`HashedCursor::seek`]. +#[derive(Debug)] +struct SeekedHashedEntry { + /// The key that was seeked. + seeked_key: B256, + /// The result of the seek. + + /// If no entry was found for the provided key, this will be [`None`]. + result: Option<(B256, V)>, +} + /// An iterator over existing intermediate branch nodes and updated leaf nodes. #[derive(Debug)] pub struct TrieNodeIter { @@ -45,19 +56,26 @@ pub struct TrieNodeIter { previous_hashed_key: Option, /// Current hashed entry. - current_hashed_entry: Option<(B256, ::Value)>, + current_hashed_entry: Option<(B256, H::Value)>, /// Flag indicating whether we should check the current walker key. should_check_walker_key: bool, + /// The last seeked hashed entry. + /// + /// We use it to not seek the same hashed entry twice, and instead re-use it. + last_seeked_hashed_entry: Option>, + #[cfg(feature = "metrics")] metrics: crate::metrics::TrieNodeIterMetrics, - #[cfg(feature = "metrics")] - previously_seeked_key: Option, + /// The key that the [`HashedCursor`] previously advanced to using [`HashedCursor::next`]. #[cfg(feature = "metrics")] previously_advanced_to_key: Option, } -impl TrieNodeIter { +impl TrieNodeIter +where + H::Value: Copy, +{ /// Creates a new [`TrieNodeIter`]. pub fn new(walker: TrieWalker, hashed_cursor: H, trie_type: TrieType) -> Self { Self { @@ -66,11 +84,10 @@ impl TrieNodeIter { previous_hashed_key: None, current_hashed_entry: None, should_check_walker_key: false, + last_seeked_hashed_entry: None, #[cfg(feature = "metrics")] metrics: crate::metrics::TrieNodeIterMetrics::new(trie_type), #[cfg(feature = "metrics")] - previously_seeked_key: None, - #[cfg(feature = "metrics")] previously_advanced_to_key: None, } } @@ -84,28 +101,39 @@ impl TrieNodeIter { /// Seeks the hashed cursor to the given key. /// + /// If the key is the same as the last seeked key, the result of the last seek is returned. + /// /// If `metrics` feature is enabled, also updates the metrics. - fn hashed_cursor_seek(&mut self, key: B256) -> Result, DatabaseError> { + fn seek_hashed_entry(&mut self, key: B256) -> Result, DatabaseError> { + if let Some(entry) = self + .last_seeked_hashed_entry + .as_ref() + .filter(|entry| entry.seeked_key == key) + .map(|entry| entry.result) + { + #[cfg(feature = "metrics")] + self.metrics.inc_leaf_nodes_same_seeked(); + return Ok(entry); + } + + let result = self.hashed_cursor.seek(key)?; + self.last_seeked_hashed_entry = Some(SeekedHashedEntry { seeked_key: key, result }); + #[cfg(feature = "metrics")] { self.metrics.inc_leaf_nodes_seeked(); - if Some(key) == self.previously_seeked_key { - self.metrics.inc_leaf_nodes_same_seeked(); - } - self.previously_seeked_key = Some(key); - if Some(key) == self.previously_advanced_to_key { self.metrics.inc_leaf_nodes_same_seeked_as_advanced(); } } - self.hashed_cursor.seek(key) + Ok(result) } /// Advances the hashed cursor to the next entry. /// /// If `metrics` feature is enabled, also updates the metrics. - fn hashed_cursor_next(&mut self) -> Result, DatabaseError> { + fn next_hashed_entry(&mut self) -> Result, DatabaseError> { let result = self.hashed_cursor.next(); #[cfg(feature = "metrics")] { @@ -122,6 +150,7 @@ impl TrieNodeIter where C: TrieCursor, H: HashedCursor, + H::Value: Copy, { /// Return the next trie node to be added to the hash builder. /// @@ -169,7 +198,7 @@ where // Set the next hashed entry as a leaf node and return trace!(target: "trie::node_iter", ?hashed_key, "next hashed entry"); - self.current_hashed_entry = self.hashed_cursor_next()?; + self.current_hashed_entry = self.next_hashed_entry()?; #[cfg(feature = "metrics")] self.metrics.inc_leaf_nodes_returned(); @@ -181,8 +210,8 @@ where Some(hashed_key) => { trace!(target: "trie::node_iter", ?hashed_key, "seeking to the previous hashed entry"); // Seek to the previous hashed key and get the next hashed entry - self.hashed_cursor_seek(hashed_key)?; - self.current_hashed_entry = self.hashed_cursor_next()?; + self.seek_hashed_entry(hashed_key)?; + self.current_hashed_entry = self.next_hashed_entry()?; } None => { // Get the seek key and set the current hashed entry based on walker's next @@ -232,7 +261,7 @@ where continue } - self.current_hashed_entry = self.hashed_cursor_seek(seek_key)?; + self.current_hashed_entry = self.seek_hashed_entry(seek_key)?; } } } @@ -470,11 +499,6 @@ mod tests { visit_type: KeyVisitType::SeekNonExact(account_3), visited_key: Some(account_3) }, - // Why do we seek the account 3 one more time? - KeyVisit { - visit_type: KeyVisitType::SeekNonExact(account_3), - visited_key: Some(account_3) - }, // Collect the siblings of the modified account KeyVisit { visit_type: KeyVisitType::Next, visited_key: Some(account_4) }, KeyVisit { visit_type: KeyVisitType::Next, visited_key: Some(account_5) },