Skip to content

perf: re-use frame allocation#2617

Closed
DaniPopes wants to merge 23 commits intobluealloy:mainfrom
DaniPopes:dani/reuse-frames
Closed

perf: re-use frame allocation#2617
DaniPopes wants to merge 23 commits intobluealloy:mainfrom
DaniPopes:dani/reuse-frames

Conversation

@DaniPopes
Copy link
Copy Markdown
Collaborator

@DaniPopes DaniPopes commented Jun 10, 2025

Generalizes #2615 to frames.

@DaniPopes DaniPopes force-pushed the dani/reuse-frames branch from f715b74 to bfc7a65 Compare June 11, 2025 08:17
@DaniPopes DaniPopes force-pushed the dani/reuse-frames branch from 0c7e036 to 7df2132 Compare June 11, 2025 10:29
@codspeed-hq
Copy link
Copy Markdown

codspeed-hq bot commented Jun 11, 2025

CodSpeed Performance Report

Merging #2617 will improve performances by 32.73%

Comparing DaniPopes:dani/reuse-frames (a2a263e) with main (0f8081b)

Summary

⚡ 136 improvements
✅ 28 untouched benchmarks

Benchmarks breakdown

Benchmark BASE HEAD Change
ADDMOD_50 35.6 µs 32 µs +11.1%
ADDRESS_50 22.3 µs 18.8 µs +18.6%
ADD_50 27.9 µs 24.4 µs +14.59%
AND_50 28 µs 24.5 µs +14.43%
BYTE_50 28.4 µs 24.7 µs +14.87%
CALLDATACOPY_50 62 µs 58.4 µs +6.13%
CALLDATALOAD_50 62.5 µs 59 µs +6%
CALLDATASIZE_50 22.2 µs 18.6 µs +19.23%
CALLER_50 22.3 µs 18.9 µs +18.24%
CALLVALUE_50 22.2 µs 18.7 µs +18.54%
CALL_50 127.3 µs 97.2 µs +30.88%
CHAINID_50 22.3 µs 18.8 µs +18.74%
CODESIZE_50 22.9 µs 18.8 µs +21.65%
COINBASE_50 22.3 µs 18.9 µs +18.05%
CREATE_50 454.2 µs 411.7 µs +10.33%
DELEGATECALL_50 121.3 µs 91.4 µs +32.73%
DIFFICULTY_50 22.6 µs 19 µs +18.52%
DIV_50 30.4 µs 26.8 µs +13.36%
DUP10_50 27.3 µs 24.4 µs +11.65%
DUP11_50 27.4 µs 24.6 µs +11.43%
... ... ... ... ...

ℹ️ Only the first 20 benchmarks are displayed. Go to the app to view all benchmarks.

@DaniPopes DaniPopes force-pushed the dani/reuse-frames branch from 51e3a9f to 0764731 Compare June 11, 2025 16:58
@DaniPopes
Copy link
Copy Markdown
Collaborator Author

This will obviously use more RAM across transactions; this breaks 32bit state tests as they now run OOM, what should we do @rakita ? we can special case 32bit to clear the map across blocks, or just ignore the test?

@rakita
Copy link
Copy Markdown
Member

rakita commented Jun 12, 2025

This will obviously use more RAM across transactions; this breaks 32bit state tests as they now run OOM, what should we do @rakita ? we can special case 32bit to clear the map across blocks, or just ignore the test?

We need to check this assumption


#[inline]
fn frame_stack<'a, F: Frame<Evm: EvmTr>>(evm: &mut F::Evm) -> &'a mut FrameStack<F> {
let f = evm.ctx_mut().local_mut().frame_stack();
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Honestly, this is not that bad, we could take a local_mut().frame_stack() inside run_exec_loop and return it back after fn finishes.

Would make run_exec_loop have a &mut FrameStack field so it is easier to handle error handles of multiple ?.

@DaniPopes DaniPopes marked this pull request as ready for review June 13, 2025 19:16
@rakita
Copy link
Copy Markdown
Member

rakita commented Jun 16, 2025

Have investigated why memory oom occurs with https://docs.rs/dhat/latest/dhat/ and have noticed that when frame_stack drops u128 values, the previously set heap allocations will not be deallocated and would leak (including boxed frame input and Bytes).

It feels the fault is with the frames in general and how they are burrowed inside Handler (hard to access), have tried a few ways to handle this better. Having frame_stack inside Evm looked the best way. Hope to have a PR for it today

@DaniPopes
Copy link
Copy Markdown
Collaborator Author

oh yeah that makes sense, i overlooked the drop

@rakita
Copy link
Copy Markdown
Member

rakita commented Jun 17, 2025

superseded by #2636

@rakita rakita closed this Jun 17, 2025
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.

2 participants