-
Notifications
You must be signed in to change notification settings - Fork 20
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
feat: calculate state root in background #197
base: yutian/fix_merge_v1.1.1
Are you sure you want to change the base?
Conversation
272a4ad
to
f170052
Compare
crates/engine/tree/src/tree/mod.rs
Outdated
.block_validation | ||
.record_state_root(&root_updates, elapsed.as_secs_f64()); | ||
|
||
self.metrics.engine.persistence_duration.record(elapsed); |
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.
seems too many duplicated code
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.
and I did see the different between new added code and the previous one
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.
you can define the oneshot::Receiver in an enum to make the code more structured and readable:
pub(crate) enum PersistenceReceiver {
/// Receiver for simple persistence tasks.
Simple(oneshot::Receiver<Option<BlockNumHash>>),
/// Receiver for persistence tasks with root updates.
WithRootUpdates(oneshot::Receiver<(Option<BlockNumHash>, TrieUpdates)>),
}
pub struct PersistenceState {
/// Hash and number of the last block persisted.
///
/// This tracks the chain height that is persisted on disk
pub(crate) last_persisted_block: BlockNumHash,
/// Receiver end of channel where the result of the persistence task will be
/// sent when done. A None value means there's no persistence task in progress.
pub(crate) rx: Option<(PersistenceReceiver, Instant)>,
/// The block above which blocks should be removed from disk, because there has been an on disk
/// reorg.
pub(crate) remove_above_state: VecDeque<u64>,
}
And use a match statement to handle different types of responses, improving readability and maintainability:
if self.persistence_state.in_progress() {
let (rx, start_time) = self
.persistence_state
.rx
.take()
.expect("If a persistence task is in progress, Receiver must be Some");
match rx {
PersistenceReceiver::Simple(mut rx) => {
// Check if the persistence task has completed
match rx.try_recv() {
Ok(last_persisted_hash_num) => {
// Handle the result of the simple persistence task
}
Err(_) => {
// Handle errors or pending state
}
}
}
PersistenceReceiver::WithRootUpdates(mut rx) => {
// Check if the persistence task with root updates has completed
match rx.try_recv() {
Ok(persistence_result) => {
// Handle the result of the persistence task with root updates
}
Err(_) => {
// Handle errors or pending state
}
}
}
}
}
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.
fixed
Description
This PR adds an option to calculate the state root in the background, improving block insertion performance during live sync.
Rationale
Calculating the state root during live sync takes a lot of time, resulting in poor performance. This feature allows for background calculation of the state root and merges calculations for multiple blocks to update the trie simultaneously. Our tests show that enabling this feature can improve performance from 50 mgasps to 400 mgasps.
Without this feature:
With this feature enabled:
Example
NA
Changes
Notable changes:
Potential Impacts