Skip to content

feat: new execution e4 for memory adapters#1733

Merged
Golovanov399 merged 55 commits into
feat/new-executionfrom
feat/new-execution-e4-memory-adapters
Jul 2, 2025
Merged

feat: new execution e4 for memory adapters#1733
Golovanov399 merged 55 commits into
feat/new-executionfrom
feat/new-execution-e4-memory-adapters

Conversation

@Golovanov399

@Golovanov399 Golovanov399 commented Jun 12, 2025

Copy link
Copy Markdown
Contributor

This resolves INT-4116.

The main change is: access adapter inventory now has (one) dense record arena that is used for all memory adapter records.
One record describes an act of either splitting a memory block into chunks of size lowest_block_size (usually equal to current address space's alignment, except maybe on initialization/finalization), or merging a memory block from chunks of size lowest_block_size, given their timestamps.

Example: one record can look like "at time 123 and address (2, 28) take a block of size 32 and merge from blocks of size 4, their timestamps were [3, 1, 4, 1, 5, 9, 2, 6]". In tracegen this record will generate one row in Adapter<32>, two rows in Adapter<16>, and four rows in Adapter<8>.

To generate trace, which we do for all adapter chips at once, we currently just iterate over all records sequentially and write into the already allocated trace matrices. We could first do a sequential scan to find, for each record, which rows in the matrices it corresponds to, and then fill them in parallel. We could also create more arenas (namely one per (block_size, lowest_block_size)), and then each pair "(arena, adapter)" defines a constant number of rows in this adapter's trace matrix per record, which would remove the need to scan first. However, this is out of scope of this PR.

Reth benchmark: https://github.com/axiom-crypto/openvm-reth-benchmark/actions/runs/16029214229

@Golovanov399 Golovanov399 force-pushed the feat/new-execution-e4-memory-adapters branch 2 times, most recently from 9036481 to 4387c0f Compare June 18, 2025 17:21
@codspeed-hq

codspeed-hq Bot commented Jun 18, 2025

Copy link
Copy Markdown

CodSpeed WallTime Performance Report

Merging #1733 will not alter performance

Comparing feat/new-execution-e4-memory-adapters (31361a7) with feat/new-execution (9fafdbb)

⚠️ Unknown Walltime execution environment detected

Using the Walltime instrument on standard Hosted Runners will lead to inconsistent data.

For the most accurate results, we recommend using CodSpeed Macro Runners: bare-metal machines fine-tuned for performance measurement consistency.

Summary

✅ 20 untouched benchmarks

@codspeed-hq

This comment was marked as outdated.

Base automatically changed from feat/new-execution-e4 to feat/new-execution June 20, 2025 09:31
@Golovanov399 Golovanov399 force-pushed the feat/new-execution-e4-memory-adapters branch from 2ec5487 to d4a85fe Compare June 23, 2025 10:41
@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@Golovanov399 Golovanov399 changed the title [wip] feat: new execution e4 for memory adapters feat: new execution e4 for memory adapters Jun 23, 2025
Comment thread crates/vm/src/system/memory/online.rs Outdated
Comment thread crates/vm/src/system/memory/online.rs Outdated
Comment thread crates/vm/src/system/memory/online.rs Outdated
jonathanpwang

This comment was marked as resolved.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@Golovanov399 Golovanov399 force-pushed the feat/new-execution-e4-memory-adapters branch from de3280d to 77e9fc5 Compare July 1, 2025 23:11
@github-actions

This comment has been minimized.

Comment thread crates/vm/src/system/memory/adapter/mod.rs Outdated
Comment thread crates/vm/src/system/memory/adapter/mod.rs Outdated
@github-actions

This comment has been minimized.

fn set_meta_block(
&mut self,
address: MemoryAddress<u32, u32>,
address_space: usize,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

don't need to change it, but using u32 instead of usize might allow the compiler to do some stuff more efficiently(?)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe the compiler should be able to find that we usually essentially do address_space as u32 so it should treat it like u32, at least most of the time, but yes, I keep in mind that we probably need to unify those usize <-> u32 conversions more

@jonathanpwang jonathanpwang left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, nice job!

@jonathanpwang

Copy link
Copy Markdown
Contributor

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@Golovanov399 Golovanov399 merged commit ffdfbbc into feat/new-execution Jul 2, 2025
27 checks passed
@Golovanov399 Golovanov399 deleted the feat/new-execution-e4-memory-adapters branch July 2, 2025 15:59
@github-actions

github-actions Bot commented Jul 2, 2025

Copy link
Copy Markdown
group app.proof_time_ms app.cycles app.cells_used leaf.proof_time_ms leaf.cycles leaf.cells_used
verify_fibair (-2 [-0.2%]) 1,264 322,699 17,271,190 - - -
fibonacci (-59 [-2.5%]) 2,324 1,500,277 50,589,231 - - -
regex 6,915 4,165,432 166,449,586 - - -
ecrecover (+31 [+2.2%]) 1,422 137,260 8,174,585 - - -
pairing (-30 [-0.7%]) 3,986 1,862,964 102,530,853 - - -

Commit: 31361a7

Benchmark Workflow

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants