Skip to content

core/vm: introduce a tracer wrapper #32273

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

omerfirmak
Copy link
Contributor

@omerfirmak omerfirmak commented Jul 25, 2025

to clean up the callsites a little and allow compile time disabling of tracing. The generic can be bubbled up to the vm.EVM and vm.EVMInterpreter to allow traced and not-traced versions of them.

@rjl493456442
Copy link
Member

allow compile time disabling of tracing.

you mean setting the tracingEnabled to false?

@omerfirmak omerfirmak force-pushed the compile-time-tracer-switch branch from 340fd66 to 4297593 Compare July 28, 2025 07:07
@omerfirmak
Copy link
Contributor Author

allow compile time disabling of tracing.

you mean setting the tracingEnabled to false?

Yeap, I was meaning that. But I replaced that with a different approach that uses generics now.

@omerfirmak omerfirmak marked this pull request as ready for review July 28, 2025 13:23
@omerfirmak omerfirmak requested a review from rjl493456442 as a code owner July 28, 2025 13:23
@fjl
Copy link
Contributor

fjl commented Jul 29, 2025

In order to measure the impact of this change in a real-world benchmark, we need to introduce the TracingSwitch flag into the actual vm interpreter loop implementation. I don't want to merge the PR without being able to quantify the improvement.

@fjl fjl removed the status:triage label Jul 29, 2025
@omerfirmak
Copy link
Contributor Author

In order to measure the impact of this change in a real-world benchmark, we need to introduce the TracingSwitch flag into the actual vm interpreter loop implementation. I don't want to merge the PR without being able to quantify the improvement.

Of course, I will follow up with that. But note that I don't see this solely as a performance optimization but something that I hope will make further optimizations (like #32160) possible with less friction.

@omerfirmak omerfirmak force-pushed the compile-time-tracer-switch branch from 4297593 to 5fac41e Compare August 5, 2025 13:47
@omerfirmak
Copy link
Contributor Author

Tested with this diff

diff --git a/core/vm/interpreter.go b/core/vm/interpreter.go
index 70ad4470d..e6d9eb156 100644
--- a/core/vm/interpreter.go
+++ b/core/vm/interpreter.go
@@ -202,7 +202,7 @@ func (in *EVMInterpreter) Run(contract *Contract, input []byte, readOnly bool) (
                gasCopy uint64 // for EVMLogger to log gas remaining before execution
                logged  bool   // deferred EVMLogger should ignore already logged steps
                res     []byte // result of the opcode execution function
-               debug   = in.evm.Config.Tracer != nil
+               debug   = in.evm.Config.Tracer != nil && tracingEnabled[TracingDisabled]()
        )
        // Don't move this deferred function, it's placed before the OnOpcode-deferred method,
        // so that it gets executed _after_: the OnOpcode needs the stacks before
omer@omer-ThinkPad-E14-Gen-3:~/Documents/go-ethereum$ benchstat runtime-check.txt compiled-check.txt 
goos: linux
goarch: amd64
pkg: github.com/ethereum/go-ethereum/core/vm/runtime
cpu: AMD Ryzen 7 5700U with Radeon Graphics         
                                       │ runtime-check.txt │         compiled-check.txt          │
                                       │      sec/op       │   sec/op     vs base                │
SimpleLoop/staticcall-identity-100M-16         281.9m ± 0%   278.7m ± 1%   -1.11% (p=0.000 n=10)
SimpleLoop/call-identity-100M-16               379.2m ± 0%   371.5m ± 0%   -2.03% (p=0.000 n=10)
SimpleLoop/loop-100M-16                        307.7m ± 1%   262.7m ± 0%  -14.62% (p=0.000 n=10)
SimpleLoop/loop2-100M-16                       273.2m ± 0%   253.8m ± 1%   -7.10% (p=0.000 n=10)
SimpleLoop/call-nonexist-100M-16               703.1m ± 0%   697.9m ± 0%   -0.74% (p=0.000 n=10)
SimpleLoop/call-EOA-100M-16                    316.7m ± 0%   312.9m ± 0%   -1.18% (p=0.000 n=10)
SimpleLoop/call-reverting-100M-16              687.4m ± 1%   684.0m ± 1%        ~ (p=0.075 n=10)
geomean                                        390.1m        374.4m        -4.03%

                                       │ runtime-check.txt │         compiled-check.txt          │
                                       │       B/op        │     B/op      vs base               │
SimpleLoop/staticcall-identity-100M-16        67.95Mi ± 0%   67.95Mi ± 0%       ~ (p=0.781 n=10)
SimpleLoop/call-identity-100M-16              107.5Mi ± 0%   107.5Mi ± 0%       ~ (p=0.778 n=10)
SimpleLoop/loop-100M-16                       5.578Ki ± 4%   5.578Ki ± 4%       ~ (p=1.000 n=10)
SimpleLoop/loop2-100M-16                      5.578Ki ± 4%   5.578Ki ± 4%       ~ (p=1.000 n=10)
SimpleLoop/call-nonexist-100M-16              165.1Mi ± 0%   165.1Mi ± 0%       ~ (p=0.796 n=10)
SimpleLoop/call-EOA-100M-16                   68.33Mi ± 0%   68.33Mi ± 0%       ~ (p=0.115 n=10)
SimpleLoop/call-reverting-100M-16             223.9Mi ± 0%   223.9Mi ± 0%       ~ (p=0.218 n=10)
geomean                                       6.604Mi        6.604Mi       +0.00%

                                       │ runtime-check.txt │         compiled-check.txt         │
                                       │     allocs/op     │  allocs/op   vs base               │
SimpleLoop/staticcall-identity-100M-16         2.740M ± 0%   2.740M ± 0%       ~ (p=0.487 n=10)
SimpleLoop/call-identity-100M-16               4.027M ± 0%   4.027M ± 0%       ~ (p=0.978 n=10)
SimpleLoop/loop-100M-16                         44.00 ± 2%    44.00 ± 2%       ~ (p=1.000 n=10)
SimpleLoop/loop2-100M-16                        44.00 ± 2%    44.00 ± 2%       ~ (p=1.000 n=10)
SimpleLoop/call-nonexist-100M-16               4.478M ± 0%   4.478M ± 0%       ~ (p=0.612 n=10)
SimpleLoop/call-EOA-100M-16                    2.239M ± 0%   2.239M ± 0%       ~ (p=0.183 n=10)
SimpleLoop/call-reverting-100M-16              5.001M ± 0%   5.001M ± 0%       ~ (p=0.239 n=10)
geomean                                        140.3k        140.3k       +0.00%

@omerfirmak omerfirmak marked this pull request as draft August 6, 2025 09:22
to clean up the callsites a little and allow compile time
disabling of tracing.
@omerfirmak omerfirmak force-pushed the compile-time-tracer-switch branch from 5fac41e to f6643b6 Compare August 6, 2025 14:39
@omerfirmak omerfirmak marked this pull request as ready for review August 6, 2025 14:40
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.

4 participants