-
Notifications
You must be signed in to change notification settings - Fork 2.1k
perf(engine): return sorted data from compute_trie_input #19340
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
base: main
Are you sure you want to change the base?
Changes from 7 commits
eb7d8c6
43ceb89
3fd4996
9b83edd
7c85ed3
4249c66
08c9a1f
23a9338
861579e
87642e4
2fbcbb4
9e1fa4a
0e2c3e7
5f51ed3
b346694
56ee954
6fd8fd2
acfb780
d3e56a2
6c5cd7c
76babb4
66c73e5
665ae71
4ca196a
2667271
dae97c6
d251dee
28b84bf
990cb15
5f90e0f
cdff945
b15a062
ba70ee7
37bdb2d
0e0e2d7
062aa27
ceb77c0
f1454d7
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 |
|---|---|---|
| @@ -1,4 +1,8 @@ | ||
| use crate::{prefix_set::TriePrefixSetsMut, updates::TrieUpdates, HashedPostState}; | ||
| use crate::{ | ||
| prefix_set::TriePrefixSetsMut, | ||
| updates::{TrieUpdates, TrieUpdatesSorted}, | ||
| HashedPostState, HashedPostStateSorted, | ||
| }; | ||
|
|
||
| /// Inputs for trie-related computations. | ||
| #[derive(Default, Debug, Clone)] | ||
|
|
@@ -118,4 +122,75 @@ impl TrieInput { | |
| self.clear(); | ||
| self | ||
| } | ||
|
|
||
| /// Converts trie input into [`TrieInputSorted`], draining the internal maps while keeping | ||
| /// their allocated capacity. | ||
| /// | ||
| /// This effectively clears all the fields in the [`TrieInput`] while preserving the | ||
| /// `HashMap` capacity for reuse. This allows us to reuse the allocated space for subsequent | ||
| /// block validations, avoiding repeated allocations and rehashing in hot paths. | ||
| /// | ||
| /// The sorted output allocates new `Vec` space, but the original `HashMap` capacity is | ||
| /// retained for the next cycle. | ||
| pub fn drain_into_sorted(&mut self) -> TrieInputSorted { | ||
yongkangc marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| TrieInputSorted { | ||
| nodes: self.nodes.drain_into_sorted(), | ||
| state: self.state.drain_into_sorted(), | ||
| prefix_sets: core::mem::take(&mut self.prefix_sets), | ||
| } | ||
| } | ||
| } | ||
|
|
||
| /// Sorted variant of [`TrieInput`] for efficient proof generation. | ||
| /// | ||
| /// This type holds sorted versions of trie data structures, which eliminates the need | ||
| /// for expensive sorting operations during multiproof generation. | ||
| #[derive(Default, Debug, Clone)] | ||
| pub struct TrieInputSorted { | ||
yongkangc marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| /// Sorted cached in-memory intermediate trie nodes. | ||
| pub nodes: TrieUpdatesSorted, | ||
| /// Sorted in-memory overlay hashed state. | ||
| pub state: HashedPostStateSorted, | ||
| /// Prefix sets for computation. | ||
| pub prefix_sets: TriePrefixSetsMut, | ||
| } | ||
|
|
||
| impl TrieInputSorted { | ||
| /// Create new sorted trie input. | ||
| pub const fn new( | ||
| nodes: TrieUpdatesSorted, | ||
| state: HashedPostStateSorted, | ||
| prefix_sets: TriePrefixSetsMut, | ||
| ) -> Self { | ||
| Self { nodes, state, prefix_sets } | ||
| } | ||
|
|
||
| /// Create from unsorted [`TrieInput`] by sorting. | ||
| pub fn from_unsorted(input: TrieInput) -> Self { | ||
| Self { | ||
| nodes: input.nodes.into_sorted(), | ||
| state: input.state.into_sorted(), | ||
| prefix_sets: input.prefix_sets, | ||
| } | ||
| } | ||
|
|
||
| /// Append state to the input by reference and extend the prefix sets. | ||
| pub fn append_ref(&mut self, state: &HashedPostState) { | ||
| self.prefix_sets.extend(state.construct_prefix_sets()); | ||
| let sorted_state = state.clone().into_sorted(); | ||
|
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. so basically we are applying sorting here, would that be a concern for performance regression? 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. if we're not using this method in this PR then I don't think we should add it. This logic made sense on TrieInput when we were not able to revert trie data, but now it's not necessary I don't think 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. makes sense, but currently we are using it here 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. I feel like theres alot more code that i can get rid off, currently still looking through / understanding the flow to see how it can be simpler. 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. I see, in this case I would have |
||
| self.state.extend_ref(&sorted_state); | ||
| } | ||
|
|
||
| /// Clear all data. | ||
| pub fn clear(&mut self) { | ||
|
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. Do we use this method? I don't think it makes much sense to use it, given that we're holding onto Arcs |
||
| self.nodes.clear(); | ||
| self.state.clear(); | ||
| self.prefix_sets.clear(); | ||
| } | ||
|
|
||
| /// Return a cleared version of this sorted trie input. | ||
| pub fn cleared(mut self) -> Self { | ||
| self.clear(); | ||
| self | ||
| } | ||
| } | ||
Uh oh!
There was an error while loading. Please reload this page.