perf(l1): remove rayon from eip-8025 execution path#6560
perf(l1): remove rayon from eip-8025 execution path#6560benbencik wants to merge 1 commit intolambdaclass:mainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR aims to eliminate rayon usage (and associated overhead) from the zkVM-guest (eip-8025) execution path by gating parallel code behind feature flags and adding sequential fallbacks.
Changes:
- Make
rayonoptional in several crates and introduce/propagateeip-8025+rayonfeature flags. - Disable
rayon-based prefetch and parallel warm/prefetch logic ineip-8025builds, with sequential fallbacks in some hot paths. - Update guest-program feature propagation so
eip-8025enables the corresponding VM feature.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| crates/vm/levm/src/db/mod.rs | Gates rayon imports and parallel prefetch methods behind cfg(all(feature = "rayon", not(feature = "eip-8025"))). |
| crates/vm/levm/Cargo.toml | Makes rayon optional; adds eip-8025 and rayon features and wires them into existing features. |
| crates/vm/backends/levm/mod.rs | Gates multiple BAL-parallel codepaths behind rayon && !eip-8025 and adds sequential fallbacks for sender-group warmup and code prefetch. |
| crates/vm/Cargo.toml | Makes rayon optional; introduces a crate-level rayon feature and propagates eip-8025. |
| crates/guest-program/Cargo.toml | Ensures eip-8025 enables ethrex-vm/eip-8025 so the VM follows the intended build mode. |
| crates/common/types/block.rs | Switches tx sender recovery to sequential iteration under eip-8025; still uses rayon for non-eip-8025. |
| crates/common/Cargo.toml | Makes rayon optional and ties it to the secp256k1 feature. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| rayon = { workspace = true, optional = true } | ||
| rkyv.workspace = true |
| structs::{Decoder, Encoder}, | ||
| }; | ||
| use ethrex_trie::Trie; | ||
| #[cfg(not(feature = "eip-8025"))] |
| // In eip-8025 builds, use sequential iteration | ||
| #[cfg(not(feature = "eip-8025"))] | ||
| return self | ||
| .transactions | ||
| .par_iter() | ||
| .map(|tx| Ok((tx, tx.sender(crypto)?))) | ||
| .collect::<Result<Vec<(&Transaction, Address)>, CryptoError>>(); |
Greptile SummaryThis PR introduces the Confidence Score: 4/5Safe to merge for all standard build configurations; one minor cfg-guard inconsistency in block.rs is unlikely to affect current builds. All findings are P2. The feature-chain plumbing is consistent across Cargo.toml files and the cfg guards in levm/mod.rs and db/mod.rs are correctly paired. The only concern is that block.rs gates the parallel branch on not(feature = eip-8025) rather than also requiring feature = secp256k1, which is less robust than the approach used elsewhere. crates/common/types/block.rs — cfg guard for rayon usage is weaker than in other crates.
|
| Filename | Overview |
|---|---|
| crates/common/types/block.rs | Adds cfg branches for par_iter vs iter in get_transactions_with_sender; the parallel branch guard is weaker than in other crates and could fail to compile in non-default feature combinations. |
| crates/vm/backends/levm/mod.rs | Gates BAL parallel execution path, execute_block_parallel, helper functions, and code-prefetch phases behind cfg(all(feature = rayon, not(feature = eip-8025))); adds correct sequential fallbacks; stale doc comment in warm_block_from_bal. |
| crates/vm/levm/src/db/mod.rs | Correctly gates prefetch_accounts and prefetch_storage overrides behind cfg(all(feature = rayon, not(feature = eip-8025))), falling back to sequential trait defaults otherwise. |
| crates/common/Cargo.toml | Makes rayon optional and ties it to the secp256k1 feature, consistent with changes in other crates. |
| crates/vm/Cargo.toml | Adds explicit rayon feature, makes rayon dep optional, and adds eip-8025 feature forwarding to ethrex-levm and ethrex-common. |
| crates/vm/levm/Cargo.toml | Makes rayon optional, adds explicit rayon feature, adds eip-8025 feature, and gates rayon behind secp256k1. |
| crates/guest-program/Cargo.toml | Adds ethrex-vm/eip-8025 to the eip-8025 feature chain so the VM crate sequential paths are activated in the guest build. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Block execution entry] --> B{feature eip-8025?}
B -- Yes --> C[Discard header_bal]
B -- No / rayon enabled --> D{header_bal present?}
D -- Yes --> E[execute_block_parallel rayon]
D -- No --> F[execute_block_sequential_no_bal rayon par_iter]
C --> G[execute_block_sequential_no_bal std iter]
E --> H[bal_to_account_updates]
F --> H
G --> I[send_state_transitions_tx]
subgraph get_transactions_with_sender
J{feature eip-8025?} -- No --> K[par_iter rayon]
J -- Yes --> L[iter sequential]
end
subgraph CachingDatabase prefetch
M{rayon AND not eip-8025?} -- Yes --> N[parallel par_iter fetch]
M -- No --> O[trait default sequential]
end
Comments Outside Diff (1)
-
crates/vm/backends/levm/mod.rs, line 1950-1953 (link)Stale doc comment describes phases as "parallel via rayon"
After this PR,
prefetch_accountsandprefetch_storagein Phases 1 and 2 fall through to the sequential default trait implementations wheneverrayonis absent oreip-8025is enabled. The doc comment still says "parallel via rayon" for both phases and should be updated to reflect the conditional behaviour.Prompt To Fix With AI
This is a comment left during a code review. Path: crates/vm/backends/levm/mod.rs Line: 1950-1953 Comment: **Stale doc comment describes phases as "parallel via rayon"** After this PR, `prefetch_accounts` and `prefetch_storage` in Phases 1 and 2 fall through to the sequential default trait implementations whenever `rayon` is absent or `eip-8025` is enabled. The doc comment still says "parallel via rayon" for both phases and should be updated to reflect the conditional behaviour. How can I resolve this? If you propose a fix, please make it concise.
Prompt To Fix All With AI
Fix the following 2 code review issues. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 2
crates/common/types/block.rs:22-23
**Rayon guard is incomplete — potential build failure without `secp256k1`**
The parallel path is gated on `#[cfg(not(feature = "eip-8025"))]`, which implicitly assumes `rayon` is always present when `eip-8025` is disabled. In `ethrex-common`, rayon is only activated as a side-effect of the `secp256k1` feature. Any build that passes `default-features = false` without explicitly enabling `secp256k1` (and without `eip-8025`) will fail to compile because the `use rayon::iter::*` import is included while `rayon` is not a dependency.
The other changed crates (`levm/mod.rs`, `db/mod.rs`) correctly use `#[cfg(all(feature = "rayon", not(feature = "eip-8025")))]`. The `secp256k1` or a dedicated `rayon` feature should be added to the condition here for consistency.
```suggestion
#[cfg(all(not(feature = "eip-8025"), feature = "secp256k1"))]
use rayon::iter::{IntoParallelRefIterator, ParallelIterator};
```
### Issue 2 of 2
crates/vm/backends/levm/mod.rs:1950-1953
**Stale doc comment describes phases as "parallel via rayon"**
After this PR, `prefetch_accounts` and `prefetch_storage` in Phases 1 and 2 fall through to the sequential default trait implementations whenever `rayon` is absent or `eip-8025` is enabled. The doc comment still says "parallel via rayon" for both phases and should be updated to reflect the conditional behaviour.
Reviews (1): Last reviewed commit: "perf(l1): remove rayon from eip-8025 exe..." | Re-trigger Greptile
| use ethrex_trie::Trie; | ||
| #[cfg(not(feature = "eip-8025"))] |
There was a problem hiding this comment.
Rayon guard is incomplete — potential build failure without
secp256k1
The parallel path is gated on #[cfg(not(feature = "eip-8025"))], which implicitly assumes rayon is always present when eip-8025 is disabled. In ethrex-common, rayon is only activated as a side-effect of the secp256k1 feature. Any build that passes default-features = false without explicitly enabling secp256k1 (and without eip-8025) will fail to compile because the use rayon::iter::* import is included while rayon is not a dependency.
The other changed crates (levm/mod.rs, db/mod.rs) correctly use #[cfg(all(feature = "rayon", not(feature = "eip-8025")))]. The secp256k1 or a dedicated rayon feature should be added to the condition here for consistency.
| use ethrex_trie::Trie; | |
| #[cfg(not(feature = "eip-8025"))] | |
| #[cfg(all(not(feature = "eip-8025"), feature = "secp256k1"))] | |
| use rayon::iter::{IntoParallelRefIterator, ParallelIterator}; |
Prompt To Fix With AI
This is a comment left during a code review.
Path: crates/common/types/block.rs
Line: 22-23
Comment:
**Rayon guard is incomplete — potential build failure without `secp256k1`**
The parallel path is gated on `#[cfg(not(feature = "eip-8025"))]`, which implicitly assumes `rayon` is always present when `eip-8025` is disabled. In `ethrex-common`, rayon is only activated as a side-effect of the `secp256k1` feature. Any build that passes `default-features = false` without explicitly enabling `secp256k1` (and without `eip-8025`) will fail to compile because the `use rayon::iter::*` import is included while `rayon` is not a dependency.
The other changed crates (`levm/mod.rs`, `db/mod.rs`) correctly use `#[cfg(all(feature = "rayon", not(feature = "eip-8025")))]`. The `secp256k1` or a dedicated `rayon` feature should be added to the condition here for consistency.
```suggestion
#[cfg(all(not(feature = "eip-8025"), feature = "secp256k1"))]
use rayon::iter::{IntoParallelRefIterator, ParallelIterator};
```
How can I resolve this? If you propose a fix, please make it concise.
Motivation
Observed that execution of the Ethrex guest program in ziskVM makes calls to
rayon. However, the execution inside the zkVM is strictly sequential, sorayonprovides no benefit and only adds runtime overhead.Description
This change uses the
eip-8025feature to control parallel execution paths. A few methods are removed from theeip-8025build, and some have a new sequential fallback that replaces the parallel iterator with standard iterator.Note: The
eip-8025continues to usestd::syncwhich is also redundant. Removing it would require a larger refactor (perhaps future work).Results
Profiling in ZisK v0.16.1 on 25 mainnet blocks 24949787 - 24949836:
1c5e518f8): 55.69BThe change decreases costs by 4-5% and reduces the complexity of the code running inside zkVM.