Skip to content

Commit b90b8d4

Browse files
7suyash7shekhirin
andauthored
perf(trie): optimize TrieNodeIter by skipping redundant seek (#15841)
Signed-off-by: 7suyash7 <[email protected]> Co-authored-by: Alexey Shekhirin <[email protected]>
1 parent bc9722d commit b90b8d4

File tree

2 files changed

+21
-25
lines changed

2 files changed

+21
-25
lines changed

crates/trie/trie/src/metrics.rs

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -83,8 +83,6 @@ pub struct TrieNodeIterMetrics {
8383
/// iterator. It does not mean the database seek was actually done, as the trie node
8484
/// iterator caches the last hashed cursor seek.
8585
leaf_nodes_same_seeked_total: Counter,
86-
/// The number of times the same leaf node as we just advanced to was seeked by the iterator.
87-
leaf_nodes_same_seeked_as_advanced_total: Counter,
8886
/// The number of leaf nodes seeked by the iterator.
8987
leaf_nodes_seeked_total: Counter,
9088
/// The number of leaf nodes advanced by the iterator.
@@ -109,11 +107,6 @@ impl TrieNodeIterMetrics {
109107
self.leaf_nodes_same_seeked_total.increment(1);
110108
}
111109

112-
/// Increment `leaf_nodes_same_seeked_as_advanced_total`.
113-
pub fn inc_leaf_nodes_same_seeked_as_advanced(&self) {
114-
self.leaf_nodes_same_seeked_as_advanced_total.increment(1);
115-
}
116-
117110
/// Increment `leaf_nodes_seeked_total`.
118111
pub fn inc_leaf_nodes_seeked(&self) {
119112
self.leaf_nodes_seeked_total.increment(1);

crates/trie/trie/src/node_iter.rs

Lines changed: 21 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -64,9 +64,10 @@ pub struct TrieNodeIter<C, H: HashedCursor> {
6464

6565
#[cfg(feature = "metrics")]
6666
metrics: crate::metrics::TrieNodeIterMetrics,
67-
/// The key that the [`HashedCursor`] previously advanced to using [`HashedCursor::next`].
68-
#[cfg(feature = "metrics")]
69-
previously_advanced_to_key: Option<B256>,
67+
/// Stores the result of the last successful [`Self::next_hashed_entry`], used to avoid a
68+
/// redundant [`Self::seek_hashed_entry`] call if the walker points to the same key that
69+
/// was just returned by `next()`.
70+
last_next_result: Option<(B256, H::Value)>,
7071
}
7172

7273
impl<C, H: HashedCursor> TrieNodeIter<C, H>
@@ -108,8 +109,7 @@ where
108109
last_seeked_hashed_entry: None,
109110
#[cfg(feature = "metrics")]
110111
metrics: crate::metrics::TrieNodeIterMetrics::new(trie_type),
111-
#[cfg(feature = "metrics")]
112-
previously_advanced_to_key: None,
112+
last_next_result: None,
113113
}
114114
}
115115

@@ -126,6 +126,18 @@ where
126126
///
127127
/// If `metrics` feature is enabled, also updates the metrics.
128128
fn seek_hashed_entry(&mut self, key: B256) -> Result<Option<(B256, H::Value)>, DatabaseError> {
129+
if let Some((last_key, last_value)) = self.last_next_result {
130+
if last_key == key {
131+
trace!(target: "trie::node_iter", seek_key = ?key, "reusing result from last next() call instead of seeking");
132+
self.last_next_result = None; // Consume the cached value
133+
134+
let result = Some((last_key, last_value));
135+
self.last_seeked_hashed_entry = Some(SeekedHashedEntry { seeked_key: key, result });
136+
137+
return Ok(result);
138+
}
139+
}
140+
129141
if let Some(entry) = self
130142
.last_seeked_hashed_entry
131143
.as_ref()
@@ -137,16 +149,13 @@ where
137149
return Ok(entry);
138150
}
139151

152+
trace!(target: "trie::node_iter", ?key, "performing hashed cursor seek");
140153
let result = self.hashed_cursor.seek(key)?;
141154
self.last_seeked_hashed_entry = Some(SeekedHashedEntry { seeked_key: key, result });
142155

143156
#[cfg(feature = "metrics")]
144157
{
145158
self.metrics.inc_leaf_nodes_seeked();
146-
147-
if Some(key) == self.previously_advanced_to_key {
148-
self.metrics.inc_leaf_nodes_same_seeked_as_advanced();
149-
}
150159
}
151160
Ok(result)
152161
}
@@ -156,12 +165,12 @@ where
156165
/// If `metrics` feature is enabled, also updates the metrics.
157166
fn next_hashed_entry(&mut self) -> Result<Option<(B256, H::Value)>, DatabaseError> {
158167
let result = self.hashed_cursor.next();
168+
169+
self.last_next_result = result.clone()?;
170+
159171
#[cfg(feature = "metrics")]
160172
{
161173
self.metrics.inc_leaf_nodes_advanced();
162-
163-
self.previously_advanced_to_key =
164-
result.as_ref().ok().and_then(|result| result.as_ref().map(|(k, _)| *k));
165174
}
166175
result
167176
}
@@ -518,12 +527,6 @@ mod tests {
518527
// Collect the siblings of the modified account
519528
KeyVisit { visit_type: KeyVisitType::Next, visited_key: Some(account_4) },
520529
KeyVisit { visit_type: KeyVisitType::Next, visited_key: Some(account_5) },
521-
// We seek the account 5 because its hash is not in the branch node, but we already
522-
// walked it before, so there should be no need for it.
523-
KeyVisit {
524-
visit_type: KeyVisitType::SeekNonExact(account_5),
525-
visited_key: Some(account_5)
526-
},
527530
KeyVisit { visit_type: KeyVisitType::Next, visited_key: None },
528531
],
529532
);

0 commit comments

Comments
 (0)