Skip to content

core/vm: remove pointers from JumpTable #32077

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

Closed
wants to merge 1 commit into from

Conversation

omerfirmak
Copy link
Contributor

no reason for them to be pointers since they are all non-nil. Remove unused "undefined" bit as well.

@omerfirmak omerfirmak requested a review from rjl493456442 as a code owner June 23, 2025 06:36
@MariusVanDerWijden MariusVanDerWijden self-assigned this Jun 26, 2025
no reason for them to be pointers since they are all non-nil.
Remove unused "undefined" bit as well.
@omerfirmak
Copy link
Contributor Author

rebased and done a quick benchmark

goos: linux
goarch: amd64
pkg: github.com/ethereum/go-ethereum/core/vm/runtime
cpu: AMD Ryzen 7 5700U with Radeon Graphics         
                                       │ ptr-jit.txt │           no-ptr-jit.txt            │
                                       │   sec/op    │   sec/op     vs base                │
SimpleLoop/staticcall-identity-100M-16   294.0m ± 0%   277.2m ± 0%   -5.72% (p=0.000 n=10)
SimpleLoop/call-identity-100M-16         338.5m ± 0%   332.4m ± 1%   -1.79% (p=0.000 n=10)
SimpleLoop/loop-100M-16                  335.0m ± 1%   289.5m ± 1%  -13.58% (p=0.000 n=10)
SimpleLoop/loop2-100M-16                 287.2m ± 1%   285.4m ± 0%        ~ (p=0.075 n=10)
SimpleLoop/call-nonexist-100M-16         663.5m ± 0%   653.3m ± 0%   -1.54% (p=0.000 n=10)
SimpleLoop/call-EOA-100M-16              271.0m ± 0%   267.9m ± 1%   -1.15% (p=0.001 n=10)
SimpleLoop/call-reverting-100M-16        532.6m ± 0%   541.6m ± 1%   +1.67% (p=0.000 n=10)
geomean                                  368.2m        355.8m        -3.37%

                                       │ ptr-jit.txt  │           no-ptr-jit.txt            │
                                       │     B/op     │     B/op      vs base               │
SimpleLoop/staticcall-identity-100M-16   67.95Mi ± 0%   67.95Mi ± 0%       ~ (p=0.950 n=10)
SimpleLoop/call-identity-100M-16         66.58Mi ± 0%   66.58Mi ± 0%  +0.00% (p=0.000 n=10)
SimpleLoop/loop-100M-16                  5.458Ki ± 8%   5.273Ki ± 4%       ~ (p=0.089 n=10)
SimpleLoop/loop2-100M-16                 5.273Ki ± 4%   5.273Ki ± 4%       ~ (p=1.000 n=10)
SimpleLoop/call-nonexist-100M-16         119.6Mi ± 0%   119.6Mi ± 0%       ~ (p=0.052 n=10)
SimpleLoop/call-EOA-100M-16              22.79Mi ± 0%   22.79Mi ± 0%       ~ (p=0.442 n=10)
SimpleLoop/call-reverting-100M-16        163.8Mi ± 0%   163.8Mi ± 0%  +0.02% (p=0.000 n=10)
geomean                                  4.761Mi        4.737Mi       -0.49%

                                       │ ptr-jit.txt │           no-ptr-jit.txt           │
                                       │  allocs/op  │  allocs/op   vs base               │
SimpleLoop/staticcall-identity-100M-16   2.740M ± 0%   2.740M ± 0%       ~ (p=0.734 n=10)
SimpleLoop/call-identity-100M-16         2.685M ± 0%   2.685M ± 0%       ~ (p=0.211 n=10)
SimpleLoop/loop-100M-16                   41.00 ± 2%    41.00 ± 2%       ~ (p=0.303 n=10)
SimpleLoop/loop2-100M-16                  41.00 ± 2%    41.00 ± 2%       ~ (p=1.000 n=10)
SimpleLoop/call-nonexist-100M-16         2.985M ± 0%   2.985M ± 0%       ~ (p=0.068 n=10)
SimpleLoop/call-EOA-100M-16              746.3k ± 0%   746.3k ± 0%       ~ (p=0.312 n=10)
SimpleLoop/call-reverting-100M-16        2.858M ± 0%   2.858M ± 0%  +0.00% (p=0.000 n=10)
geomean                                  96.64k        96.64k       +0.00%

Copy link
Member

@lightclient lightclient left a comment

Choose a reason for hiding this comment

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

Got slightly different benchmark:

goos: linux
goarch: amd64
pkg: github.com/ethereum/go-ethereum/core/vm/runtime
cpu: 12th Gen Intel(R) Core(TM) i7-1260P
                                       │  old.bench  │             new.bench              │
                                       │   sec/op    │   sec/op     vs base               │
SimpleLoop/staticcall-identity-100M-16   174.3m ± 2%   177.0m ± 2%       ~ (p=0.052 n=10)
SimpleLoop/call-identity-100M-16         206.4m ± 1%   204.0m ± 1%  -1.15% (p=0.015 n=10)
SimpleLoop/loop-100M-16                  164.8m ± 2%   163.9m ± 1%  -0.53% (p=0.029 n=10)
SimpleLoop/loop2-100M-16                 212.8m ± 2%   207.0m ± 2%  -2.73% (p=0.000 n=10)
SimpleLoop/call-nonexist-100M-16         454.1m ± 1%   471.3m ± 1%  +3.79% (p=0.000 n=10)
SimpleLoop/call-EOA-100M-16              185.4m ± 2%   181.5m ± 1%  -2.10% (p=0.011 n=10)
SimpleLoop/call-reverting-100M-16        348.3m ± 2%   342.3m ± 1%  -1.72% (p=0.000 n=10)
geomean                                  232.7m        231.7m       -0.44%

                                       │   old.bench   │               new.bench               │
                                       │     B/op      │     B/op       vs base                │
SimpleLoop/staticcall-identity-100M-16   67.95Mi ±  0%    67.95Mi ± 0%   -0.00% (p=0.001 n=10)
SimpleLoop/call-identity-100M-16         66.58Mi ±  0%    66.58Mi ± 0%   -0.00% (p=0.000 n=10)
SimpleLoop/loop-100M-16                  4.853Ki ±  0%    4.459Ki ± 3%   -8.11% (p=0.000 n=10)
SimpleLoop/loop2-100M-16                 5.456Ki ± 14%    4.894Ki ± 3%        ~ (p=0.283 n=10)
SimpleLoop/call-nonexist-100M-16         96.83Mi ±  0%   119.59Mi ± 0%  +23.51% (p=0.000 n=10)
SimpleLoop/call-EOA-100M-16              22.79Mi ±  0%    22.78Mi ± 0%   -0.01% (p=0.000 n=10)
SimpleLoop/call-reverting-100M-16        163.9Mi ±  0%    163.8Mi ± 0%   -0.03% (p=0.000 n=10)
geomean                                  4.565Mi          4.576Mi        +0.25%

                                       │  old.bench  │              new.bench              │
                                       │  allocs/op  │  allocs/op   vs base                │
SimpleLoop/staticcall-identity-100M-16   2.740M ± 0%   2.740M ± 0%   -0.00% (p=0.001 n=10)
SimpleLoop/call-identity-100M-16         2.685M ± 0%   2.685M ± 0%   -0.00% (p=0.002 n=10)
SimpleLoop/loop-100M-16                   40.00 ± 0%    40.00 ± 3%        ~ (p=1.000 n=10)
SimpleLoop/loop2-100M-16                  40.00 ± 3%    41.00 ± 2%   +2.50% (p=0.002 n=10)
SimpleLoop/call-nonexist-100M-16         2.239M ± 0%   2.985M ± 0%  +33.33% (p=0.000 n=10)
SimpleLoop/call-EOA-100M-16              746.3k ± 0%   746.3k ± 0%   -0.00% (p=0.000 n=10)
SimpleLoop/call-reverting-100M-16        2.858M ± 0%   2.858M ± 0%   -0.00% (p=0.000 n=10)
geomean                                  92.10k        96.30k        +4.56%

@@ -42,9 +42,6 @@ type operation struct {

// memorySize returns the memory size required for the operation
memorySize memorySizeFunc

// undefined denotes if the instruction is not officially defined in the jump table
undefined bool
Copy link
Member

Choose a reason for hiding this comment

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

This was added in #30418 and Martin wasn't the happiest about it. IIRC it was used for EOF, so should be okay to remove.

@omerfirmak
Copy link
Contributor Author

Yeah, I think I messed up my original benchmark. I'm consistently seeing %13 slower now, instead of %13 faster like the original benchmark suggested. I think I know why. I'll fold this into my next PR.

@omerfirmak omerfirmak closed this Jul 9, 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.

3 participants