Skip to content

Remove resolved perf-check TODOs from UInt256 shift helpers#10406

Open
parthdagia05 wants to merge 1 commit intobesu-eth:mainfrom
parthdagia05:feat/uint256-shift-perf-todos
Open

Remove resolved perf-check TODOs from UInt256 shift helpers#10406
parthdagia05 wants to merge 1 commit intobesu-eth:mainfrom
parthdagia05:feat/uint256-shift-perf-todos

Conversation

@parthdagia05
Copy link
Copy Markdown

@parthdagia05 parthdagia05 commented May 4, 2026

What

Remove the two // TODO: check perf - wiring [shiftRight|shiftLeft] callers with this one comments above UInt256.sar0 and UInt256.shl0, added in #10216.

Why

The TODOs asked whether routing the public shift methods (sar, shr, shl) through the new internal helpers (sar0, shl0) preserves performance versus the pre-refactor implementation in StackArithmetic. I ran the existing V2 shift JMH benchmarks before and after #10216 to find out.

Method

  • PRE state: 8e4ef80c40^ (commit 67dbd9a4c5, parent of Move SAR, SHR and SHL to UInt256 #10216)
  • POST state: upstream/main at 80b381ee9f
  • Benchmarks: SarOperationBenchmarkV2, ShlOperationBenchmarkV2, ShrOperationBenchmarkV2
  • JMH config: 3 forks × (3 warmup + 5 measurement iterations × 1s) repo default
  • Hardware: local x86_64 laptop, JDK 21
  • Significance: marked when 99.9% confidence intervals do not overlap

Results

Benchmark              Scenario              PRE (ns/op)     POST (ns/op)      Δ%      Significant?
----------------------------------------------------------------------------------------------------
SarOperationBenchmarkV2  SHIFT_0               4.953 ± 0.140   5.486 ± 0.461   +10.8%  noise
SarOperationBenchmarkV2  NEGATIVE_SHIFT_1      7.522 ± 0.387   7.445 ± 0.348    -1.0%  noise
SarOperationBenchmarkV2  ALL_BITS_SHIFT_1      7.363 ± 0.251   7.238 ± 0.435    -1.7%  noise
SarOperationBenchmarkV2  POSITIVE_SHIFT_1      7.083 ± 0.279   6.891 ± 0.335    -2.7%  noise
SarOperationBenchmarkV2  NEGATIVE_SHIFT_128    5.698 ± 0.107   5.830 ± 0.145    +2.3%  noise
SarOperationBenchmarkV2  NEGATIVE_SHIFT_255    5.942 ± 0.163   6.116 ± 0.232    +2.9%  noise
SarOperationBenchmarkV2  POSITIVE_SHIFT_128    5.649 ± 0.107   6.099 ± 0.143    +8.0%  REGRESSION
SarOperationBenchmarkV2  POSITIVE_SHIFT_255    5.655 ± 0.259   5.868 ± 0.204    +3.8%  noise
SarOperationBenchmarkV2  OVERFLOW_SHIFT_256    5.608 ± 0.233   6.045 ± 0.544    +7.8%  noise
SarOperationBenchmarkV2  OVERFLOW_LARGE_SHIFT  5.785 ± 0.196   5.939 ± 0.215    +2.7%  noise
SarOperationBenchmarkV2  FULL_RANDOM          16.628 ± 1.596  17.642 ± 1.630    +6.1%  noise
ShlOperationBenchmarkV2  SHIFT_0               4.903 ± 0.235   5.352 ± 0.260    +9.2%  noise
ShlOperationBenchmarkV2  SHIFT_1               7.465 ± 0.262   7.391 ± 0.401    -1.0%  noise
ShlOperationBenchmarkV2  SHIFT_128             5.869 ± 0.139   5.835 ± 0.152    -0.6%  noise
ShlOperationBenchmarkV2  SHIFT_255             5.944 ± 0.230   5.982 ± 0.200    +0.6%  noise
ShlOperationBenchmarkV2  OVERFLOW_SHIFT_256    5.058 ± 0.140   5.779 ± 0.192   +14.3%  REGRESSION
ShlOperationBenchmarkV2  OVERFLOW_LARGE_SHIFT  5.220 ± 0.143   5.913 ± 0.217   +13.3%  REGRESSION
ShlOperationBenchmarkV2  FULL_RANDOM          12.975 ± 1.183  11.084 ± 0.608   -14.6%  IMPROVEMENT
ShrOperationBenchmarkV2  SHIFT_0               4.993 ± 0.202   5.240 ± 0.264    +4.9%  noise
ShrOperationBenchmarkV2  SHIFT_1               7.341 ± 0.375   6.990 ± 0.365    -4.8%  noise
ShrOperationBenchmarkV2  SHIFT_128             5.514 ± 0.164   5.903 ± 0.150    +7.1%  REGRESSION
ShrOperationBenchmarkV2  SHIFT_255             5.663 ± 0.176   5.892 ± 0.168    +4.0%  noise
ShrOperationBenchmarkV2  OVERFLOW_SHIFT_256    5.079 ± 0.128   5.710 ± 0.197   +12.4%  REGRESSION
ShrOperationBenchmarkV2  OVERFLOW_LARGE_SHIFT  5.227 ± 0.136   5.984 ± 0.142   +14.5%  REGRESSION
ShrOperationBenchmarkV2  FULL_RANDOM          12.658 ± 1.378  10.826 ± 0.742   -14.5%  noise

Summary: 6 statistically significant regressions (+7%–+14%, all sub-1ns absolute) on overflow / specific fixed-shift cases, 1 statistically significant improvement (SHL FULL_RANDOM, −14.6%), 18 within noise.

Conclusion

The refactor is performance-neutral on realistic workloads (random-shift is unchanged or faster). The small overflow-path regressions are sub-nanosecond and not worth a follow-up at this absolute scale. The TODOs have been answered closing them.

A separate question worth a follow-up issue (not this PR): whether re-introducing public-method early returns for shift >= 256 would recover the OVERFLOW regression without bloating the API.

Test plan

  • ./gradlew :evm:compileJava passes
  • :evm:test - UInt256Test (91), UInt256PropertyBasedTest (118), Shift256OperationsTest (7), ShiftOperationsPropertyBasedTest (10), ShiftOperationsV2PropertyBasedTest (25) - 251 tests, all pass

Signed-off-by: Parth Dagia <parth.24bcs10414@sst.scaler.com>
@parthdagia05
Copy link
Copy Markdown
Author

@lu-pinto @ahamlat

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.

1 participant