Repository navigation
Conversation
buildBytecodeCache called resolveJumpTargetsByAbstractStack twice in a row with the same arguments: once inside the buildJumpDestMap profile region and once right after it, clearing the first result in between. The function reads only the bytecode, the jump-destination map and the metrics table, and writes only Cache.ResolvedJumpTargets, so the second call rebuilt an identical map. The duplicate dates from DTVMStack#462, whose comment already says the pass runs once. Drop the second call. On 338 mainnet contracts (5.1 MB of bytecode) all six cache fields are identical before and after, with EnableSPP off and on, and the total cache build time falls from 135.3 ms to 104.9 ms (-22.5%) with SPP off and from 147.0 ms to 114.8 ms (-21.9%) with SPP on. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
⚡ Performance Regression Check Results✅ Performance Check Passed (interpreter)Performance Benchmark Results (threshold: 25%)
Summary: 194 benchmarks, 0 regressions ✅ Performance Check Passed (multipass)Performance Benchmark Results (threshold: 25%)
Summary: 194 benchmarks, 0 regressions |
This was referenced Oct 9, 2026
Contributor
Author
|
Closing in favour of #615, which removes the same duplicate call. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
1. Does this PR affect any open issues?(Y/N) and add issue references (e.g. "fix #123", "re #123".):
2. What is the scope of this PR (e.g. component or file name):
buildBytecodeCacheinsrc/evm/evm_cache.cpp.3. Provide a description of the PR(e.g. more details, effects, motivations or doc link):
buildBytecodeCachecallsresolveJumpTargetsByAbstractStacktwice in a row with the same arguments: once inside thebuildJumpDestMapprofile region, and once right after it, clearing the first result in between. The function reads only the bytecode, the jump-destination map and the metrics table, and writes onlyCache.ResolvedJumpTargets, so the second call rebuilds an identical map. The duplicate dates from #462, whose comment already says the pass runs once.This PR deletes the second call (6 lines). Nothing else changes.
Measured on
mainate839f6b, Release build, on 338 mainnet contracts (5.1 MB of runtime bytecode, 23 B to 24,575 B each):buildBytecodeCache, total over the 338 contractsEnableSPP = falseEnableSPP = trueEach value is the median of 30 in-process repetitions (5 processes, 6 repetitions each after one warm-up) on one pinned core. The before and after ranges do not overlap: [131.8, 136.2] against [100.9, 105.3] ms with SPP off, and [144.5, 148.0] against [113.1, 115.3] ms with SPP on.
The draft #615 removes the same call as part of a larger restructuring of this function. This PR is that one deletion on its own; whichever of the two lands second needs a trivial conflict resolution at this spot.
No change document: the PR restores the run-once behaviour the code comment already describes and adds no capability.
4. Are there any breaking changes?(Y/N) and describe the breaking changes(e.g. more details, motivations or doc link):
5. Are there test cases for these changes?(Y/N) select and add more details, references or doc links:
No new test: the removed call has no observable effect other than time. Checked on this branch:
EnableSPPoff and on, a hash over all sixEVMBytecodeCachefields (JumpDestMap,PushValueMap,GasChunkEnd,GasChunkCost,GasChunkCostSPP, andResolvedJumpTargetsin sorted order) is the same onmainand on this branch: 676 of 676 comparisons, covering 220,603 resolved jump targets per setting.ctestin a Release multipass build withDTVM_TEST_REVISION=Cancun: 14/14 with default arguments, and 14/14 withSPEC_TESTS_ARGSset as in the CI multipass job. This includesevmCacheTests(16 cases),evmInterpTests(275),evmJitFrontendTests(182),evmDifferentialTests(79) andevmStateTests(1,805).tools/run_evm_tests.py -r build/dtvm --format evmwith-m multipass --enable-evm-gasand with-m interpreter: 0 failures.clang-format15.0.7 with--dry-run -Werroron the changed file, andgit diff --check.Benchmark and equivalence method: a temporary gtest case, not part of this PR, reads the 338 bytecode files, calls
buildBytecodeCacheon each, and prints the field hash and the total time per pass. The same case was built intomainand into this branch.6. Release note
🤖 Generated with Claude Code