-
Notifications
You must be signed in to change notification settings - Fork 2.5k
perf(storage): batch trie updates across blocks in save_blocks #21142
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.
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
Changes from 3 commits
c022186
f3e0b67
308e05f
f76a85f
89c36bc
764d5d5
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -555,11 +555,24 @@ impl<TX: DbTx + DbTxMut + 'static, N: NodeTypesForProvider> DatabaseProvider<TX, | |
| let start = Instant::now(); | ||
| self.write_hashed_state(&trie_data.hashed_state)?; | ||
| timings.write_hashed_state += start.elapsed(); | ||
| } | ||
| } | ||
|
|
||
| let start = Instant::now(); | ||
| self.write_trie_updates_sorted(&trie_data.trie_updates)?; | ||
| timings.write_trie_updates += start.elapsed(); | ||
| // Write all trie updates in a single batch. | ||
| // This reduces cursor open/close overhead from N calls to 1. | ||
| // Uses hybrid algorithm: extend_ref for small batches, k-way merge for large. | ||
| if save_mode.with_state() { | ||
| let start = Instant::now(); | ||
| // Collect Arc refs first to extend their lifetime | ||
| let trie_updates: Vec<_> = blocks.iter().map(|b| b.trie_updates()).collect(); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. same here re collect. check for perf regression compared to previous |
||
| // merge_batch_hybrid expects newest-to-oldest, so reverse the iterator | ||
| let merged = TrieUpdatesSorted::merge_batch_hybrid( | ||
| trie_updates.iter().rev().map(|arc| arc.as_ref()), | ||
| ); | ||
| if !merged.is_empty() { | ||
| self.write_trie_updates_sorted(&merged)?; | ||
| } | ||
| timings.write_trie_updates += start.elapsed(); | ||
| } | ||
|
|
||
| // Full mode: update history indices | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -690,6 +690,47 @@ impl HashedPostStateSorted { | |
| Self { accounts, storages } | ||
| } | ||
|
|
||
| /// Hybrid batch-merge sorted hashed post states. Iterator yields **newest to oldest**. | ||
| /// | ||
| /// Uses a hybrid algorithm that switches between `extend_ref` (for small batches) | ||
| /// and k-way `merge_batch` (for large batches) based on benchmarked crossover point. | ||
| /// | ||
| /// - Small k (< threshold): O(n * k) via `extend_ref` loop, but with low constant factors | ||
| /// - Large k (≥ threshold): O(n log k) via k-way merge | ||
| /// | ||
| /// The threshold is tuned based on benchmarks where `extend_ref` wins up to ~64 items. | ||
| pub fn merge_batch_hybrid<'a>(states: impl IntoIterator<Item = &'a Self>) -> Self { | ||
| const MERGE_BATCH_THRESHOLD: usize = 64; | ||
|
|
||
| let states: Vec<_> = states.into_iter().collect(); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. we can skip this collect again because we collected before this alrready? |
||
|
|
||
| if states.is_empty() { | ||
| return Self::default(); | ||
| } | ||
|
|
||
| if states.len() == 1 { | ||
| return states[0].clone(); | ||
| } | ||
|
|
||
| if states.len() < MERGE_BATCH_THRESHOLD { | ||
| // Small k: extend_ref loop is faster. | ||
| // States are newest-to-oldest, so iterate in reverse (oldest-to-newest) | ||
| // to let newer values override older ones. | ||
| let mut iter = states.iter().rev(); | ||
| let first = iter.next().expect("states is non-empty"); | ||
| let mut result = (*first).clone(); | ||
|
|
||
| for state in iter { | ||
| result.extend_ref(state); | ||
| } | ||
|
|
||
| result | ||
| } else { | ||
| // Large k: merge_batch is faster (O(n log k) via k-way merge) | ||
| Self::merge_batch(states) | ||
| } | ||
| } | ||
|
|
||
| /// Clears all accounts and storage data. | ||
| pub fn clear(&mut self) { | ||
| self.accounts.clear(); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -686,6 +686,47 @@ impl TrieUpdatesSorted { | |
|
|
||
| Self { account_nodes, storage_tries } | ||
| } | ||
|
|
||
| /// Hybrid batch-merge sorted trie updates. Iterator yields **newest to oldest**. | ||
| /// | ||
| /// Uses a hybrid algorithm that switches between `extend_ref` (for small batches) | ||
| /// and k-way `merge_batch` (for large batches) based on benchmarked crossover point. | ||
| /// | ||
| /// - Small k (< threshold): O(n * k) via `extend_ref` loop, but with low constant factors | ||
| /// - Large k (≥ threshold): O(n log k) via k-way merge | ||
| /// | ||
| /// The threshold is tuned based on benchmarks where `extend_ref` wins up to ~64 items. | ||
| pub fn merge_batch_hybrid<'a>(updates: impl IntoIterator<Item = &'a Self>) -> Self { | ||
| const MERGE_BATCH_THRESHOLD: usize = 64; | ||
|
|
||
| let updates: Vec<_> = updates.into_iter().collect(); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. same here, i think we can prob remove this collect |
||
|
|
||
| if updates.is_empty() { | ||
| return Self::default(); | ||
| } | ||
|
|
||
| if updates.len() == 1 { | ||
| return updates[0].clone(); | ||
| } | ||
|
|
||
| if updates.len() < MERGE_BATCH_THRESHOLD { | ||
| // Small k: extend_ref loop is faster. | ||
| // Updates are newest-to-oldest, so iterate in reverse (oldest-to-newest) | ||
| // to let newer values override older ones. | ||
| let mut iter = updates.iter().rev(); | ||
| let first = iter.next().expect("updates is non-empty"); | ||
| let mut result = (*first).clone(); | ||
|
|
||
| for update in iter { | ||
| result.extend_ref(update); | ||
| } | ||
|
|
||
| result | ||
| } else { | ||
| // Large k: merge_batch is faster (O(n log k) via k-way merge) | ||
| Self::merge_batch(updates) | ||
| } | ||
| } | ||
| } | ||
|
|
||
| impl AsRef<Self> for TrieUpdatesSorted { | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not sure if we can remove a collect here?