cannon: allocator preheap instrumentation [rebased from ec2#2]#16030
cannon: allocator preheap instrumentation [rebased from ec2#2]#16030Inphi merged 60 commits intoethereum-optimism:developfrom
Conversation
|
I jumped in a bit. The stale cache issues seem to have gone with the changed testing, but will still probably want to add limiting the instruction set and heap size to the tests. Ill wait for the rest to get merge, looks like fuzz tests are left, and can probably add the code/heap size VMfactory options in the difftester.go file after and remove the StoreInstructionWithCacheUpdate function I added earlier that helped with the stale cache issues. Will wait for confirmation that test migration is done. Thank you! |
Yes - just need to get this last PR merged and then the migration will be done and tests will be stable again 🙏 |
Update - this PR is merged now! |
Thanks. Let's rebase the migration changes from develop. |
|
/ci authorize 98ae85b |
…incremented, added decoded cache updateing to difftester_test.go
|
Missed integrating difftester_test.go with changes to cache and instr/heap size. Some tests that failed in instrumented_test.go I got working locally after slightly increasing the max step limit. |
|
/ci authorize 25b4751 |
|
/ci authorize 11b2176 |
|
Im not sure how to approach the multithreaded test, looks like there is a timeout in the ci tests, the tests seem to individually pass, other than one test causes an OS signal kill for me locally. For the fuzz tests, they all pass on my local implementation, Im not sure how to approach the error. |
The tests are OOM'ing in CI as the test runner doesn't have much memory to work with. Lemme figure something out.. |
* 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.
The fp tests now require alot of memory to run. Particularly when cannon is used. So limit parallelism to avoid OOM.
|
@salindne Check out this branch that addresses the CI failures - https://github.com/ethereum-optimism/optimism/compare/inphi/ec2/alloc-preheap The following adjustments were made to ensure that cannon is usable in CI:
If this all looks good to you, I can merge those changes into this PR and we should be good to merge to develop. I tried creating a PR on the Chainsafe/optimism targeting your branch but was unable to do so. |
|
@Inphi Looked through and looks good to me. Please do merge. |
|
@salindne I don't have permission to push to your branch. I'm not sure why this didn't work before, but I've created a PR with the changes - ChainSafe#10. Could you please merge that in. |
cannon: Ensure FP tests are runnable in CI
![almanax-ai[bot]](https://avatars.githubusercontent.com/in/934833?s=60&v=4){kind=small}
|
/ci authorize ce9dc7e |
e7efd8b
* 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>
…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>
6 participants
This PR revives ec2#2, originally opened by Eric from ChainSafe, for upstream consideration. It includes instrumentation for runtime allocator preheap behavior useful in constrained deployments of the OP Stack.
Changes were migrated to ChainSafe/optimism#ec2/alloc-preheap after Eric left our organization.
cc @clabby for context — reviving Eric's previous PR (#2 on ec2/optimism) now ported to ChainSafe's fork here. Will try to resolve conflicts