Slice Access to Regions of Memory + Pre-Decode Program #2
Closed
ec2 wants to merge 11 commits intoec2/bitlistfrom
Closed
Slice Access to Regions of Memory + Pre-Decode Program #2ec2 wants to merge 11 commits intoec2/bitlistfrom
ec2 wants to merge 11 commits intoec2/bitlistfrom
Conversation
|
@clabby could you please chime in on this approach to memory lookups for a program like Kona? Any surprises? |
I didn't read into this too deeply, but I think this would cause issues if it's considering the entirety of the loadable ELF sections as code sections. Kona slab-allocates its heap at compile time, and never |
|
Yeah we should be able to make this work with kona, just needs some modifications to the kona's program headers: The final |
|
ethereum-optimism#14606 has been merged! I think this PR was just waiting for that, does it need anything else? |
|
@ec2 would you be able to make this PR against the optimism repo? |
Owner
Author
|
Hey sorry @pauldowman I'm bad at github notification 😅 . I am no longer working at Chainsafe I'm at Risc0. I will transfer this branch over to our friends to get this over the finish line |
|
@pauldowman - we now have this pr on the monorepo: ethereum-optimism#16030 |
github-merge-queue bot
pushed a commit
to ethereum-optimism/optimism
that referenced
this pull request
Aug 18, 2025
* Abstract the Merkle representation * Implement asterisc's MPT * migrate tests for mpt * migrate tests for mpt * copied benchmarks from asterisc * fix failed merge * Avoid pagelookup twice during setword invalidation * use uints * hashpool * alloc pages upfront * remove clutter * fix * gomallocregion * more readable mapped regions * fix state json codec test * fix for singlethread too * fix op-challenger test * Remove MPT implementation * address comments * fix benchmark * Use uint64 and also reuse hasher and buffers * clean * clean up some * fix range * Address reviewer feedback: clarify region bounds and add instruction cache fallback * Revert "Address reviewer feedback: clarify region bounds and add instruction cache fallback" This reverts commit 2a19b22. * Clarify region bounds and add fallback to memory decode if PC exceeds instruction cache * Update cannon/mipsevm/multithreaded/mips.go Co-authored-by: Inphi <mlaw2501@gmail.com> * fixed size of preheap cache for binaryTreeMemory * slice out of bounds fixed in GetWord and AllocPage, pagetest syntax fix * fix program heap initialization for binary tree * moved cache decoded from instrumented state declaration to doMipsStep * added default and set region map size to newMemory * added 4096 byte memory region option to all testing cases using VMFactory * clean up * cache_decode to be initialize on first mipstep only for single step test compatibility * added heap region sizing for unit/fuzz tests * Revert " cache_decode to be initialize on first mipstep only for single step test compatibility" This reverts commit 47a7166. * cache allocation in instrumentedState initialization, catch for stale cache in doMipsStep * remove unit test logic from domipstep and added helper for cache update in tests * merge migration changes from develop * some test step boundaries in instrumented_test needed to be slightly incremented, added decoded cache updateing to difftester_test.go * small changes to step count and mismatch error vs string fault * cannon: Use tiny i-cache by default * uses a small i-cache by default to accomodate unit tests. This prevents tests from running out of memory in CI. * fixes fuzz tests to preload the i-cache with the test instruction. * op-e2e: Limit max parallelism for fp tests The fp tests now require alot of memory to run. Particularly when cannon is used. So limit parallelism to avoid OOM. * increase parallelism --------- Co-authored-by: Eric Tu <tu.eric@hotmail.com> Co-authored-by: Inphi <mlaw2501@gmail.com>
janjakubnanista
pushed a commit
to ethereum-optimism/optimism
that referenced
this pull request
Aug 19, 2025
* Abstract the Merkle representation * Implement asterisc's MPT * migrate tests for mpt * migrate tests for mpt * copied benchmarks from asterisc * fix failed merge * Avoid pagelookup twice during setword invalidation * use uints * hashpool * alloc pages upfront * remove clutter * fix * gomallocregion * more readable mapped regions * fix state json codec test * fix for singlethread too * fix op-challenger test * Remove MPT implementation * address comments * fix benchmark * Use uint64 and also reuse hasher and buffers * clean * clean up some * fix range * Address reviewer feedback: clarify region bounds and add instruction cache fallback * Revert "Address reviewer feedback: clarify region bounds and add instruction cache fallback" This reverts commit 2a19b22. * Clarify region bounds and add fallback to memory decode if PC exceeds instruction cache * Update cannon/mipsevm/multithreaded/mips.go Co-authored-by: Inphi <mlaw2501@gmail.com> * fixed size of preheap cache for binaryTreeMemory * slice out of bounds fixed in GetWord and AllocPage, pagetest syntax fix * fix program heap initialization for binary tree * moved cache decoded from instrumented state declaration to doMipsStep * added default and set region map size to newMemory * added 4096 byte memory region option to all testing cases using VMFactory * clean up * cache_decode to be initialize on first mipstep only for single step test compatibility * added heap region sizing for unit/fuzz tests * Revert " cache_decode to be initialize on first mipstep only for single step test compatibility" This reverts commit 47a7166. * cache allocation in instrumentedState initialization, catch for stale cache in doMipsStep * remove unit test logic from domipstep and added helper for cache update in tests * merge migration changes from develop * some test step boundaries in instrumented_test needed to be slightly incremented, added decoded cache updateing to difftester_test.go * small changes to step count and mismatch error vs string fault * cannon: Use tiny i-cache by default * uses a small i-cache by default to accomodate unit tests. This prevents tests from running out of memory in CI. * fixes fuzz tests to preload the i-cache with the test instruction. * op-e2e: Limit max parallelism for fp tests The fp tests now require alot of memory to run. Particularly when cannon is used. So limit parallelism to avoid OOM. * increase parallelism --------- Co-authored-by: Eric Tu <tu.eric@hotmail.com> Co-authored-by: Inphi <mlaw2501@gmail.com>
leopoldjoy
pushed a commit
to leopoldjoy/optimism
that referenced
this pull request
Aug 22, 2025
…eum-optimism#16030) * Abstract the Merkle representation * Implement asterisc's MPT * migrate tests for mpt * migrate tests for mpt * copied benchmarks from asterisc * fix failed merge * Avoid pagelookup twice during setword invalidation * use uints * hashpool * alloc pages upfront * remove clutter * fix * gomallocregion * more readable mapped regions * fix state json codec test * fix for singlethread too * fix op-challenger test * Remove MPT implementation * address comments * fix benchmark * Use uint64 and also reuse hasher and buffers * clean * clean up some * fix range * Address reviewer feedback: clarify region bounds and add instruction cache fallback * Revert "Address reviewer feedback: clarify region bounds and add instruction cache fallback" This reverts commit 2a19b22. * Clarify region bounds and add fallback to memory decode if PC exceeds instruction cache * Update cannon/mipsevm/multithreaded/mips.go Co-authored-by: Inphi <mlaw2501@gmail.com> * fixed size of preheap cache for binaryTreeMemory * slice out of bounds fixed in GetWord and AllocPage, pagetest syntax fix * fix program heap initialization for binary tree * moved cache decoded from instrumented state declaration to doMipsStep * added default and set region map size to newMemory * added 4096 byte memory region option to all testing cases using VMFactory * clean up * cache_decode to be initialize on first mipstep only for single step test compatibility * added heap region sizing for unit/fuzz tests * Revert " cache_decode to be initialize on first mipstep only for single step test compatibility" This reverts commit 47a7166. * cache allocation in instrumentedState initialization, catch for stale cache in doMipsStep * remove unit test logic from domipstep and added helper for cache update in tests * merge migration changes from develop * some test step boundaries in instrumented_test needed to be slightly incremented, added decoded cache updateing to difftester_test.go * small changes to step count and mismatch error vs string fault * cannon: Use tiny i-cache by default * uses a small i-cache by default to accomodate unit tests. This prevents tests from running out of memory in CI. * fixes fuzz tests to preload the i-cache with the test instruction. * op-e2e: Limit max parallelism for fp tests The fp tests now require alot of memory to run. Particularly when cannon is used. So limit parallelism to avoid OOM. * increase parallelism --------- Co-authored-by: Eric Tu <tu.eric@hotmail.com> Co-authored-by: Inphi <mlaw2501@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Memory accesses are expensive mainly due to the fact that we have to access go's map. Indexing into a slice is much better. This method however does mean that we need to alloc more memory upfront. But since Cannon is tailored towards op-program, this shouldnt be a problem because the memory access pattern is predictable because we know how go's runtime allocates memory.
Because we know where the program section lies, we can also decode all the instructions before the VM starts, which according to measurements, will decrease reads into memory by 70%+.