-
Notifications
You must be signed in to change notification settings - Fork 122
perf(l1,l2): parallel merkleization (top-down 16-ways) #5377
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
This implements a Gravity-like top-down 16-ways parallel Merkleization, based on our own pipeline work but extending to shard the updates and join the updated tries at the end.
Lines of code reportTotal lines added: Detailed view |
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.
Pull Request Overview
This PR implements a 16-way parallel merkleization strategy for Ethereum state trie updates, based on a top-down sharding approach. The goal is to improve performance by distributing the merkleization work across multiple worker threads, each responsible for updating a specific shard of the trie based on the first nibble of hashed addresses.
Key Changes:
- Introduced parallel merkleization with 16 worker threads, each handling accounts whose hashed addresses start with a specific nibble (0-F)
- Refactored
handle_merkleizationto coordinate worker threads and merge results - Added
handle_merkleization_subtrieto process updates within each shard - Modified
process_incoming_update_messageto accept pre-hashed addresses and return void instead of the state root hash
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| for (choice, worker) in workers_handles.into_iter().enumerate() { | ||
| let worker_result = worker | ||
| .join() | ||
| .map_err(|e| StoreError::Custom(format!("join failed: {e:?}",)))??; | ||
| let Some(root_node) = worker_result.state_updates.get(&Nibbles::default()) else { | ||
| continue; | ||
| }; | ||
| let root_node = Node::decode(root_node)?; | ||
| match root_node { | ||
| Node::Branch(mut branch) => { | ||
| real_root.choices[choice] = std::mem::take(&mut branch.choices[choice]); | ||
| } | ||
| Node::Extension(_extension) => { | ||
| todo!() | ||
| } | ||
| Node::Leaf(_leaf) => { | ||
| todo!() | ||
| } | ||
| } | ||
| code_updates.extend(worker_result.code_updates); | ||
| storage_updates_map.extend(worker_result.storage_updates); | ||
| state_updates_map.extend(worker_result.state_updates); |
Copilot
AI
Nov 17, 2025
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.
When a worker receives no updates for its shard, it's skipped with continue, leaving real_root.choices[choice] as the default empty NodeRef. This is incorrect - the subtrie should be the parent's subtrie, not empty.
To fix this, we need to ensure that even when a worker has no updates, we still copy the corresponding child from the parent root node, or ensure workers always produce a root node even with zero updates.
| for (choice, worker) in workers_handles.into_iter().enumerate() { | |
| let worker_result = worker | |
| .join() | |
| .map_err(|e| StoreError::Custom(format!("join failed: {e:?}",)))??; | |
| let Some(root_node) = worker_result.state_updates.get(&Nibbles::default()) else { | |
| continue; | |
| }; | |
| let root_node = Node::decode(root_node)?; | |
| match root_node { | |
| Node::Branch(mut branch) => { | |
| real_root.choices[choice] = std::mem::take(&mut branch.choices[choice]); | |
| } | |
| Node::Extension(_extension) => { | |
| todo!() | |
| } | |
| Node::Leaf(_leaf) => { | |
| todo!() | |
| } | |
| } | |
| code_updates.extend(worker_result.code_updates); | |
| storage_updates_map.extend(worker_result.storage_updates); | |
| state_updates_map.extend(worker_result.state_updates); | |
| // Decode the parent root node so we can copy unchanged subtries | |
| let parent_root_node = self.state_store.get_node(&parent_header.state_root)? | |
| .ok_or(StoreError::Custom("parent root node not found".to_string()))?; | |
| let parent_root = match Node::decode(&parent_root_node)? { | |
| Node::Branch(branch) => branch, | |
| _ => return Err(StoreError::Custom("parent root is not a branch node".to_string())), | |
| }; | |
| for (choice, worker) in workers_handles.into_iter().enumerate() { | |
| let worker_result = worker | |
| .join() | |
| .map_err(|e| StoreError::Custom(format!("join failed: {e:?}",)))??; | |
| if let Some(root_node) = worker_result.state_updates.get(&Nibbles::default()) { | |
| let root_node = Node::decode(root_node)?; | |
| match root_node { | |
| Node::Branch(mut branch) => { | |
| real_root.choices[choice] = std::mem::take(&mut branch.choices[choice]); | |
| } | |
| Node::Extension(_extension) => { | |
| todo!() | |
| } | |
| Node::Leaf(_leaf) => { | |
| todo!() | |
| } | |
| } | |
| code_updates.extend(worker_result.code_updates); | |
| storage_updates_map.extend(worker_result.storage_updates); | |
| state_updates_map.extend(worker_result.state_updates); | |
| } else { | |
| // No updates for this shard; copy from parent root | |
| real_root.choices[choice] = parent_root.choices[choice].clone(); | |
| } |
crates/blockchain/blockchain.rs
Outdated
| todo!() | ||
| } | ||
| Node::Leaf(_leaf) => { | ||
| todo!() |
Copilot
AI
Nov 17, 2025
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.
Incomplete implementation for Node::Extension case. This will panic at runtime if a worker returns an Extension node as the root. According to the FIXME comment on line 349, this assumes the state root is always a branch node. However, for robustness, this case should be handled (perhaps by returning an error with a clear message rather than panicking with todo!()).
| todo!() | |
| } | |
| Node::Leaf(_leaf) => { | |
| todo!() | |
| return Err(StoreError::Custom( | |
| "Worker returned Extension node as root; expected Branch node. This is not supported.".to_string(), | |
| )); | |
| } | |
| Node::Leaf(_leaf) => { | |
| return Err(StoreError::Custom( | |
| "Worker returned Leaf node as root; expected Branch node. This is not supported.".to_string(), | |
| )); |
crates/blockchain/blockchain.rs
Outdated
| todo!() | ||
| } | ||
| Node::Leaf(_leaf) => { | ||
| todo!() |
Copilot
AI
Nov 17, 2025
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.
Incomplete implementation for Node::Leaf case. This will panic at runtime if a worker returns a Leaf node as the root. According to the FIXME comment on line 349, this assumes the state root is always a branch node. However, for robustness, this case should be handled (perhaps by returning an error with a clear message rather than panicking with todo!()).
| todo!() | |
| return Err(StoreError::Custom( | |
| "Worker returned a Leaf node as the root, which is not supported. Expected a Branch node.".to_string(), | |
| )); |
Benchmark Block Execution Results Comparison Against Main
|
This implements a Gravity-like top-down 16-ways parallel Merkleization,
based on our own pipeline work but extending to shard the updates and
join the updated tries at the end.