-
Notifications
You must be signed in to change notification settings - Fork 21k
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
Conversation
no reason for them to be pointers since they are all non-nil. Remove unused "undefined" bit as well.
rebased and done a quick benchmark
|
There was a problem hiding this 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 |
There was a problem hiding this comment.
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.
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. |
no reason for them to be pointers since they are all non-nil. Remove unused "undefined" bit as well.