feat(interpreter): Unify instruction fn signature #283
Conversation
958bddf
to
fb8073d
Compare
|
new run (with microbench) old run: There is not a lot that can be seen from flamegraph, but there they are: flamegraph new: flamegraph old: |
|
OS: Mac OS Test info for rakita/unify_instr_fn: Test info for main: |
There was a problem hiding this comment.
Love this PR as it has a ton of useful changes. Could we please isolate them in separate PRs, starting with the bench change, followed by the individual opcode changes, followed by the function signature unification? That way we'll know for sure what change caused what performance gain.
|
thanks for the review @gakonst! I experimented with this idea (and a few others) 3 months ago and got pretty decent results, this was one of them. Will try to play with additional dispatch strategies but I am assuming that benefit will not be significant but would potentially allow more creative ways to use revm (as in injecting the instruction fn, I will need to see how it goes). So this pull request is what I expected to happen it was not surprising. |
|
That makes sense - your PR of course, no strong opinions just wanted to take a close look. Love the outcome. Can we add the new benchmarks to the evm-bench repo? |
|
@ziyadedher can you rerun https://github.com/ziyadedher/evm-bench with latest main? ^^ |
|
@gakonst ooh cool! Yeah, will do tonight. |
|
I have one more round of small optimizations, but this should give a good boost in performance. |
|
I also see ~10% bumps on snailtracer! But interestingly, I don't see any noticeable differences on other benches, see ziyadedher/evm-bench@aad8de9 This might make sense since the optimization is primarily in dispatches and whatnot it seems. Just wanted to float the results! |
|
I assumed it would be more, around 15%. Yeah, it is mostly dispatch optimization and interpreter loop, I will play with it a little bit more in future but I don't expect a lot. revm is a little bit different from other evm's as it has both the Host and Interpreter together. This means that for test execution, SSLOAD and SSTORE access two HashMaps to find value, one for substate and one for cached value, If I remove this (planned here: #307), test for revm will be more accurate. |
All opcode instructions are now functions with same signature:
pub fn jumpi(interpreter: &mut Interpreter, _host: &mut dyn Host)This is revm internal change and it does not affect external API.
This allows the compiler to optimize calls in the interpreter hot loop and it gives around ~10% faster
snailtracerexecution on i7. I am interested to see if the same boost can be found on macs withMprocessors.This is one of steps needed to define multiple dispatch strategies related to interpreter loop. As instructions now have same signature they can be put in an array, this can have potentially new ways to use revm as it is easier to just switch function pointer to something else (custom function to sload/sstore?)
Specwas simplified andis_staticis moved as variable to interpreter (Previously it was inside Spec)There was one regression with ruint in casting U256 to saturated usize here. It was faster to just check limbs.
microbenchadded torevm-tests