Skip to content

GROOVY-12065: Implement peephole optimization for bytecode generation#2591

Draft
daniellansun wants to merge 1 commit into
masterfrom
GROOVY-12065
Draft

GROOVY-12065: Implement peephole optimization for bytecode generation#2591
daniellansun wants to merge 1 commit into
masterfrom
GROOVY-12065

Conversation

@daniellansun

Copy link
Copy Markdown
Contributor

This comment was marked as outdated.

@asf-gitbox-commits asf-gitbox-commits force-pushed the GROOVY-12065 branch 2 times, most recently from 377c68b to d1aeb66 Compare June 6, 2026 13:39
@daniellansun daniellansun changed the title Implement peephole optimization for constant loading in bytecode generation GROOVY-12065: Implement peephole optimization for constant loading in bytecode generation Jun 6, 2026

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Performance Alert ⚠️

Possible performance regression was detected for benchmark.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 1.50.

Benchmark suite Current: 2a55a90 Previous: 5499e6c Ratio
org.apache.groovy.bench.dispatch.CallsiteBench.dispatch_3_polymorphic_java 6381.848447984659 ops/ms 2061.672284547263 ops/ms 3.10
org.apache.groovy.bench.dispatch.CallsiteBench.dispatch_8_megamorphic_java 6026.931429013922 ops/ms 1557.261659873728 ops/ms 3.87
org.apache.groovy.bench.AckermannBench.java ( {"n":"5"} ) 0.10029679168967505 ms/op 0.05051886991481147 ms/op 1.99
org.apache.groovy.bench.AckermannBench.java ( {"n":"6"} ) 0.4584048799045104 ms/op 0.2130785099853715 ms/op 2.15
org.apache.groovy.bench.AckermannBench.java ( {"n":"7"} ) 1.9316165215476144 ms/op 0.9758806863895909 ms/op 1.98
org.apache.groovy.bench.AryBench.groovyCS ( {"n":"1000"} ) 0.1382858471799125 ms/op 0.057643790485718605 ms/op 2.40
org.apache.groovy.bench.AryBench.groovyCS ( {"n":"1000000"} ) 267.78055662500003 ms/op 114.49380287669935 ms/op 2.34
org.apache.groovy.bench.AryBench.java ( {"n":"1000"} ) 0.13760981117671897 ms/op 0.08895810367001782 ms/op 1.55
org.apache.groovy.bench.AryBench.java ( {"n":"1000000"} ) 268.15117947500005 ms/op 112.86152071045751 ms/op 2.38

This comment was automatically generated by workflow using github-action-benchmark.

@codecov-commenter

codecov-commenter commented Jun 6, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 87.17949% with 50 lines in your changes missing coverage. Please review.
✅ Project coverage is 68.3569%. Comparing base (c92ba5f) to head (2a55a90).

Files with missing lines Patch % Lines
.../classgen/asm/PeepholeOptimizingMethodVisitor.java 87.2629% 31 Missing and 16 partials ⚠️
...rg/codehaus/groovy/classgen/AsmClassGenerator.java 81.8182% 2 Missing ⚠️
...org/codehaus/groovy/classgen/asm/OperandStack.java 83.3333% 1 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@                Coverage Diff                 @@
##               master      #2591        +/-   ##
==================================================
+ Coverage     68.3072%   68.3569%   +0.0497%     
- Complexity      33391      33524       +133     
==================================================
  Files            1518       1519         +1     
  Lines          126871     127222       +351     
  Branches        23000      23026        +26     
==================================================
+ Hits            86662      86965       +303     
- Misses          32526      32558        +32     
- Partials         7683       7699        +16     
Files with missing lines Coverage Δ
...g/codehaus/groovy/classgen/asm/BytecodeHelper.java 87.1935% <100.0000%> (-0.7518%) ⬇️
...va/org/codehaus/groovy/classgen/asm/MopWriter.java 85.8974% <100.0000%> (+0.1831%) ⬆️
...org/codehaus/groovy/classgen/asm/OperandStack.java 77.6398% <83.3333%> (-0.9730%) ⬇️
...rg/codehaus/groovy/classgen/AsmClassGenerator.java 85.1852% <81.8182%> (+0.0324%) ⬆️
.../classgen/asm/PeepholeOptimizingMethodVisitor.java 87.2629% <87.2629%> (ø)

... and 5 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@github-actions

github-actions Bot commented Jun 6, 2026

Copy link
Copy Markdown
Contributor

JMH summary — classic (commit 0cda4cd)

Speedup vs trailing 90-day baseline on gh-pages. Higher = faster.
1.00 = in line with history. Per-benchmark ratio, geomean within group.
Time-per-op units inverted so direction is consistent.

Group Speedup n
bench 1.018 × 26
core 0.979 × 77
grails 0.985 × 80

Baseline: dev/bench/jmh/<part>/classic/data.js on gh-pages, trailing 90 days. Daily dashboard · Per-suite raw data

@github-actions

github-actions Bot commented Jun 6, 2026

Copy link
Copy Markdown
Contributor

JMH summary — indy (commit 0cda4cd)

Speedup vs trailing 90-day baseline on gh-pages. Higher = faster.
1.00 = in line with history. Per-benchmark ratio, geomean within group.
Time-per-op units inverted so direction is consistent.

Group Speedup n
bench 1.000 × 26
core 0.991 × 77
grails 1.019 × 80

Baseline: dev/bench/jmh/<part>/indy/data.js on gh-pages, trailing 90 days. Daily dashboard · Per-suite raw data

@daniellansun daniellansun changed the title GROOVY-12065: Implement peephole optimization for constant loading in bytecode generation GROOVY-12065: Implement peephole optimization for bytecode generation Jun 6, 2026
@daniellansun daniellansun marked this pull request as draft June 6, 2026 16:51
@daniellansun daniellansun self-assigned this Jun 6, 2026
@testlens-app

testlens-app Bot commented Jun 6, 2026

Copy link
Copy Markdown

✅ All tests passed ✅

⚠️ TestLens detected flakiness ⚠️

Test Summary

Check Project/Task Test Runs
Build and test / lts (17, macos-latest) :test ActorTest > testScheduleAtFixedRateCancelStopsFurtherFires() ❌ 🚫

🏷️ Commit: 2a55a90
▶️ Tests: 103752 executed
⚪️ Checks: 60/60 completed


Learn more about TestLens at testlens.app.

@sonarqubecloud

sonarqubecloud Bot commented Jun 6, 2026

Copy link
Copy Markdown

Comment on lines +575 to +576
} else if (isPrimitiveLong(type) || isPrimitiveFloat(type) || isPrimitiveDouble(type)) {
mv.visitLdcInsn(value);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Doesn't this require LCONST_0 and so on so the operand stack type is correct?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Maybe it was different when most of the bytecode code was written, but current version of asm actually handle the special case internally: https://asm.ow2.io/javadoc/org/objectweb/asm/MethodVisitor.html#visitLdcInsn(java.lang.Object) - AFAIK

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.

5 participants