refactor(ethexe/processor): split run module into separate files#5267
refactor(ethexe/processor): split run module into separate files#5267
Conversation
- Extract chunks_splitting logic to dedicated module - Extract chunk_execution_spawn logic to dedicated module - Extract chunk_execution_processing logic to dedicated module - Improve code organization and maintainability
playX18
added
A0-pleasereview
|
Review changes with Changed Files
|
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request refactors the ethexe processor's run module by decomposing it into smaller, more focused files. This change improves the modularity of the codebase, making it easier to navigate and maintain the logic responsible for splitting, spawning, and processing execution chunks. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request refactors the ethexe/processor/src/handling/run module by extracting its internal logic into dedicated files: chunk_execution_processing.rs, chunk_execution_spawn.rs, and chunks_splitting.rs. This reorganization improves code structure and maintainability. The review feedback suggests a more idiomatic approach in collect_chunk_journals by using fold instead of map to avoid side effects when calculating the maximum gas spent and collecting journals.
| let mut max_gas_spent_in_chunk = 0u64; | ||
|
|
||
| let chunk_journals = chunk_outputs | ||
| .into_iter() | ||
| .map( | ||
| |(program_id, new_state_hash, program_journals, gas_spent)| { | ||
| // Handle state updates that occurred during journal processing within the runtime (allocations, pages). | ||
| // This should happen before processing the journal notes because `send_dispatch` from another program can modify the state. | ||
| ctx.inner_mut().transitions.modify(program_id, |state, _| { | ||
| state.hash = new_state_hash; | ||
| }); | ||
|
|
||
| max_gas_spent_in_chunk = max_gas_spent_in_chunk.max(gas_spent); | ||
|
|
||
| (program_id, program_journals) | ||
| }, | ||
| ) | ||
| .collect(); | ||
|
|
||
| (chunk_journals, max_gas_spent_in_chunk) |
There was a problem hiding this comment.
For better maintainability and adherence to functional programming principles, it's preferable to avoid side effects within a map closure. The current implementation mutates max_gas_spent_in_chunk from within the map.
Using fold would be more idiomatic here. It can build the chunk_journals vector and calculate the maximum gas spent in a single, side-effect-free pass.
| let mut max_gas_spent_in_chunk = 0u64; | |
| let chunk_journals = chunk_outputs | |
| .into_iter() | |
| .map( | |
| |(program_id, new_state_hash, program_journals, gas_spent)| { | |
| // Handle state updates that occurred during journal processing within the runtime (allocations, pages). | |
| // This should happen before processing the journal notes because `send_dispatch` from another program can modify the state. | |
| ctx.inner_mut().transitions.modify(program_id, |state, _| { | |
| state.hash = new_state_hash; | |
| }); | |
| max_gas_spent_in_chunk = max_gas_spent_in_chunk.max(gas_spent); | |
| (program_id, program_journals) | |
| }, | |
| ) | |
| .collect(); | |
| (chunk_journals, max_gas_spent_in_chunk) | |
| chunk_outputs.into_iter().fold( | |
| (Vec::with_capacity(chunk_outputs.len()), 0u64), | |
| |(mut chunk_journals, max_gas_spent_in_chunk), | |
| (program_id, new_state_hash, program_journals, gas_spent)| { | |
| // Handle state updates that occurred during journal processing within the runtime (allocations, pages). | |
| // This should happen before processing the journal notes because `send_dispatch` from another program can modify the state. | |
| ctx.inner_mut().transitions.modify(program_id, |state, _| { | |
| state.hash = new_state_hash; | |
| }); | |
| chunk_journals.push((program_id, program_journals)); | |
| ( | |
| chunk_journals, | |
| max_gas_spent_in_chunk.max(gas_spent), | |
| ) | |
| }, | |
| ) |
1 participant
Resolves #5120
@gear-tech/dev