Conversation
msooseth
left a comment
There was a problem hiding this comment.
In general, I'm OK with such optimizations in case they benefit Echidna speed. Symbolic execution speed is mostly governed by SMT solver speed, so as long as this works for you, I'm good with it. However, it'd be nice to post a bench measurement test to the text of the PR so when we wanna understand things in the future, we can look it up.
|
Perhaps @blishko wants to bench-test this too? Either way, it'd be important to have some bench test numbers here as part of the documentation for this change, though? |
|
hm interesting! I just re-benchmarked this over #1047 and it tells a very different story. I suppose GC involvement makes it hard to measure accurately 🤔 Let's hold off merging for now cc @blishko bench-perf |
Adding {-# INLINE getOp #-} enables GHC's case-of-case transformation,
eliminating the intermediate GenericOp constructor allocation in opcode
dispatch. This also lets GHC sink expensive thunks (contract lookups,
fee schedule fields) into only the opcode branches that use them.
Add doBenchmark = true to shellFor so that benchmark dependencies (tasty-bench) are available in nix develop without cabal update. Replace deprecated testTarget with testTargets.
|
hm I don't know what was going on with my system before 🤔 I rebased and re-ran the benchmark here and it doesn't give me the horrible numbers from before anymore. But it's still not a clear win. bench-perf 4c1013f vs 2baa264 (main) |
blishko
left a comment
There was a problem hiding this comment.
On my computer I see very slight improvement on bench-perf.
I think it is OK to include this change.
|
I have arm Mac. |
|
For completeness, I am adding numbers from my machine: DetailsI was also eyeballing the numbers from main and on average, numbers from this branch tended to be slightly better, even though the framework evaluates it as "same as baseline". |
|
@msooseth, Are you OK with merging? If so, can you please dismiss the "Requested changes" review? |
Description
This PR inlines getOp. I see a measurable speed improvement with this change locally. The optimization was suggested by Claude Code.
Checklist