-
Notifications
You must be signed in to change notification settings - Fork 2.1k
perf: reuse accounts trie in payload processing #16181
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
Conversation
648cd41 to
5bd4e41
Compare
5bd4e41 to
613ea79
Compare
21b17c5 to
6043a1a
Compare
crates/trie/sparse/src/trie.rs
Outdated
| Blind { | ||
| /// This is an optional field that can be used to store a previously allocated | ||
| /// trie. In these cases, the trie will still be treated as blind, but the allocated trie | ||
| /// will be reused if the trie becomes revealed. | ||
| allocated: Option<Box<RevealedSparseTrie<P>>>, | ||
| }, |
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.
let's get rid of this pls. maybe make it a separate variant? e.g. AllocatedEmpty
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.
sure
crates/trie/sparse/src/trie.rs
Outdated
| /// This is an optional field that can be used to store a previously allocated | ||
| /// trie. In these cases, the trie will still be treated as blind, but the allocated trie | ||
| /// will be reused if the trie becomes revealed. | ||
| allocated: Option<Box<RevealedSparseTrie<P>>>, |
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.
it doesn't make sense to keep entire revealed sparse trie. let's only keep nodes and values hash maps and any other stuff that we are interested in. can be extracted into SparseTrieNodes or SparseTrieState struct or smth
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.
made SparseTrieState and put nodes, values, and mask hashmaps
ea688fc to
f0758aa
Compare
f0758aa to
5e65e9b
Compare
rkrasiuk
left a comment
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.
thanks, i like this much more
Co-authored-by: graphite-app[bot] <96075541+graphite-app[bot]@users.noreply.github.com>
Co-authored-by: Alexey Shekhirin <[email protected]> Co-authored-by: graphite-app[bot] <96075541+graphite-app[bot]@users.noreply.github.com>
This sends the accounts trie back to the payload processor whenever the sparse trie finishes.
depends on #16179
Previously

reserve_nodestook ~30% of time in revealingMain profile: https://share.firefox.dev/3S18zep
There are now almost no

reserve_nodescalls in account revealing, and the only insertion-related calls take a very small percentage of time.Branch profile: https://share.firefox.dev/456JBlv