You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@groovy.apache.org by "daniellansun (via GitHub)" <gi...@apache.org> on 2026/06/06 13:06:02 UTC
[PR] Implement peephole optimization for constant loading in bytecode generation [groovy]
daniellansun opened a new pull request, #2591:
URL: https://github.com/apache/groovy/pull/2591
https://issues.apache.org/jira/browse/GROOVY-12065
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@groovy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GROOVY-12065: Implement peephole optimization for constant loading in bytecode generation [groovy]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #2591:
URL: https://github.com/apache/groovy/pull/2591#issuecomment-4639259214
### JMH summary — classic (commit `94f84ea`)
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 | 0.998 × | 26 |
| core | 1.000 × | 77 |
| grails | 0.982 × | 80 |
<sub>Baseline: <code>dev/bench/jmh/<part>/classic/data.js</code> on gh-pages, trailing 90 days. <a href="https://apache.github.io/groovy/dev/bench/jmh/summary.html">Daily dashboard</a> · <a href="https://apache.github.io/groovy/dev/bench/jmh/">Per-suite raw data</a></sub>
<!-- jmh-summary:classic -->
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@groovy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GROOVY-12065: Implement peephole optimization for bytecode generation [groovy]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #2591:
URL: https://github.com/apache/groovy/pull/2591#issuecomment-4639703467
## [](https://sonarcloud.io/dashboard?id=apache_groovy&pullRequest=2591) **Quality Gate passed**
Issues
 [5 New issues](https://sonarcloud.io/project/issues?id=apache_groovy&pullRequest=2591&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_groovy&pullRequest=2591&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_groovy&pullRequest=2591&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [89.2% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_groovy&pullRequest=2591&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_groovy&pullRequest=2591&metric=new_duplicated_lines_density&view=list)
<!-- sqra-placement-anchor -->
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_groovy&pullRequest=2591)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@groovy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Implement peephole optimization for constant loading in bytecode generation [groovy]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #2591:
URL: https://github.com/apache/groovy/pull/2591#discussion_r3367384202
##########
src/main/java/org/codehaus/groovy/classgen/AsmClassGenerator.java:
##########
@@ -669,13 +671,13 @@ protected void visitConstructorOrMethod(final MethodNode node, final boolean isC
Statement code = node.getCode();
BytecodeInstruction instruction; // fast path for getters, setters, etc.
if (code instanceof BytecodeSequence && (instruction = ((BytecodeSequence) code).getBytecodeInstruction()) != null) {
- instruction.visit(mv);
+ instruction.visit(codeVisitor);
} else {
visitStdMethod(node, isConstructor, parameters, code);
}
try {
- mv.visitMaxs(0, 0);
+ codeVisitor.visitMaxs(0, 0);
Review Comment:
Since the controller’s active MethodVisitor is now the `codeVisitor` wrapper, `visitCode()` should be invoked on `codeVisitor` as well (not directly on `mv`). Calling `visitCode()` on a different visitor than the one receiving instructions can break wrapper invariants if `PeepholeOptimizingMethodVisitor` (or future wrappers) ever need to observe `visitCode()`/`visitEnd()` pairing.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@groovy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GROOVY-12065: Implement peephole optimization for bytecode generation [groovy]
Posted by "paulk-asert (via GitHub)" <gi...@apache.org>.
paulk-asert merged PR #2591:
URL: https://github.com/apache/groovy/pull/2591
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@groovy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Implement peephole optimization for constant loading in bytecode generation [groovy]
Posted by "testlens-app[bot] (via GitHub)" <gi...@apache.org>.
testlens-app[bot] commented on PR #2591:
URL: https://github.com/apache/groovy/pull/2591#issuecomment-4638909176
## 🚨 TestLens detected 1 failed test 🚨
Here is what you can do:
1) Inspect the test failures carefully.
2) If you are convinced that some of the tests are flaky, you can mute them below.
3) Finally, trigger a rerun by checking the rerun checkbox.
### Test Summary
| Check | Project/Task | Test | Runs |
|---|---|---|---|
| [Build and test / lts \(17, macos-latest\)](https://github.com/apache/groovy/actions/runs/27063343479/job/79879818487?pr=2591) | :test | ActorTest > testScheduleAtFixedRateCancelStopsFurtherFires\(\) | ❌ |
🏷️ Commit: 377c68b20763d3bd7fb8daeedc103048af276022
▶️ Tests: 60369 executed
🟡 Checks: 14/50 completed
### Test Failures
<details>
<summary><strong>ActorTest > testScheduleAtFixedRateCancelStopsFurtherFires()</strong> (:test in <a href="https://github.com/apache/groovy/actions/runs/27063343479/job/79879818487?pr=2591">Build and test / lts (17, macos-latest)</a>)</summary>
```
java.lang.AssertionError: expected 2 or 3 ticks (got 4). Expression: (n in [2, 3]). Values: n = 4
at org.codehaus.groovy.runtime.InvokerHelper.createAssertError(InvokerHelper.java:402)
at TestScript20.run(TestScript20.groovy:28)
at groovy.lang.GroovyShell.evaluate(GroovyShell.java:553)
at groovy.lang.GroovyShell.evaluate(GroovyShell.java:588)
at groovy.lang.GroovyShell.evaluate(GroovyShell.java:572)
at groovy.test.GroovyAssert.assertScript(GroovyAssert.java:106)
at groovy.test.GroovyAssert.assertScript(GroovyAssert.java:96)
at groovy.concurrent.ActorTest.testScheduleAtFixedRateCancelStopsFurtherFires(ActorTest.groovy:1372)
```
</details>
### Muted Tests
> [!NOTE]
> Checks are currently running using the configuration below.
Select tests to mute in this pull request:
🔲 ActorTest > testScheduleAtFixedRateCancelStopsFurtherFires\(\) <!-- uniqueId=[engine:junit-jupiter]/[class:groovy.concurrent.ActorTest]/[method:testScheduleAtFixedRateCancelStopsFurtherFires()] -->
Reuse successful test results:
🔲 ♻️ Only rerun the tests that failed or were muted before
Click the checkbox to trigger a rerun:
🔲 **Rerun jobs**
---
_Learn more about TestLens at [testlens.app](https://testlens.app)._
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@groovy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GROOVY-12065: Implement peephole optimization for constant loading in bytecode generation [groovy]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #2591:
URL: https://github.com/apache/groovy/pull/2591#issuecomment-4639247198
## [](https://sonarcloud.io/dashboard?id=apache_groovy&pullRequest=2591) **Quality Gate passed**
Issues
 [4 New issues](https://sonarcloud.io/project/issues?id=apache_groovy&pullRequest=2591&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_groovy&pullRequest=2591&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_groovy&pullRequest=2591&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [93.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_groovy&pullRequest=2591&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_groovy&pullRequest=2591&metric=new_duplicated_lines_density&view=list)
<!-- sqra-placement-anchor -->
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_groovy&pullRequest=2591)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@groovy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GROOVY-12065: Implement peephole optimization for constant loading in bytecode generation [groovy]
Posted by "testlens-app[bot] (via GitHub)" <gi...@apache.org>.
testlens-app[bot] commented on PR #2591:
URL: https://github.com/apache/groovy/pull/2591#issuecomment-4639292761
## ✅ All tests passed ✅
🏷️ Commit: 043340d7bff41bda81fa8a80c1c97597a3486eb7
▶️ Tests: 134036 executed
⚪️ Checks: 60/60 completed
---
_Learn more about TestLens at [testlens.app](https://testlens.app)._
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@groovy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GROOVY-12065: Implement peephole optimization for bytecode generation [groovy]
Posted by "testlens-app[bot] (via GitHub)" <gi...@apache.org>.
testlens-app[bot] commented on PR #2591:
URL: https://github.com/apache/groovy/pull/2591#issuecomment-4879767200
## 🚨 TestLens detected 1 failed test 🚨
Here is what you can do:
1) Inspect the test failures carefully.
2) If you are convinced that some of the tests are flaky, you can mute them below.
3) Finally, trigger a rerun by checking the rerun checkbox.
### Test Summary
#### [Build and test / lts \(17, macos-latest\)](https://github.com/apache/groovy/actions/runs/28686886754/job/85081128254?pr=2591) > :test
| Test | Runs |
|---|---|
| Groovy11046 > testMissingDependency2\(\) | ❌ |
🏷️ Commit: 298263db0d853a31bffdd9dcd10f26c039c370df
▶️ Tests: 15449 executed
🟡 Checks: 3/57 completed
### Test Failures
<details>
<summary><strong>Groovy11046 > testMissingDependency2()</strong> (:test in <a href="https://github.com/apache/groovy/actions/runs/28686886754/job/85081128254?pr=2591">Build and test / lts (17, macos-latest)</a>)</summary>
```
java.lang.AssertionError: script should have failed at runtime. Expression: (err.asString() =~ at org.apache.logging.log4j.LogManager.getLogger\()
at org.codehaus.groovy.runtime.InvokerHelper.createAssertError(InvokerHelper.java:402)
at bugs.Groovy11046.testMissingDependency2(Groovy11046.groovy:52)
```
</details>
### Muted Tests
> [!NOTE]
> Checks are currently running using the configuration below.
Select tests to mute in this pull request:
🔲 Groovy11046 > testMissingDependency2\(\) <!-- uniqueId=[engine:junit-jupiter]/[class:bugs.Groovy11046]/[method:testMissingDependency2()] -->
Reuse successful test results:
🔲 ♻️ Only rerun the tests that failed or were muted before
Click the checkbox to trigger a rerun:
🔲 **Rerun jobs**
---
_Learn more about TestLens at [testlens.app](https://testlens.app)._
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@groovy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GROOVY-12065: Implement peephole optimization for bytecode generation [groovy]
Posted by "testlens-app[bot] (via GitHub)" <gi...@apache.org>.
testlens-app[bot] commented on PR #2591:
URL: https://github.com/apache/groovy/pull/2591#issuecomment-4881106095
## ✅ All tests passed ✅
🏷️ Commit: d61c051517fb168eced19d49409094b88c7ce77c
▶️ Tests: 88421 executed
⚪️ Checks: 31/31 completed
---
_Learn more about TestLens at [testlens.app](https://testlens.app)._
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@groovy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GROOVY-12065: Implement peephole optimization for bytecode generation [groovy]
Posted by "testlens-app[bot] (via GitHub)" <gi...@apache.org>.
testlens-app[bot] commented on PR #2591:
URL: https://github.com/apache/groovy/pull/2591#issuecomment-4639832270
## 🚨 TestLens detected 1 failed test 🚨
Here is what you can do:
1) Inspect the test failures carefully.
2) If you are convinced that some of the tests are flaky, you can mute them below.
3) Finally, trigger a rerun by checking the rerun checkbox.
### Test Summary
| Check | Project/Task | Test | Runs |
|---|---|---|---|
| [Build and test / lts \(17, macos-latest\)](https://github.com/apache/groovy/actions/runs/27068603029/job/79893700299?pr=2591) | :test | ActorTest > testScheduleAtFixedRateCancelStopsFurtherFires\(\) | ❌ |
🏷️ Commit: 452530587f875ef655c13f1a2a4149d9485c38e1
▶️ Tests: 78931 executed
🟡 Checks: 8/56 completed
### Test Failures
<details>
<summary><strong>ActorTest > testScheduleAtFixedRateCancelStopsFurtherFires()</strong> (:test in <a href="https://github.com/apache/groovy/actions/runs/27068603029/job/79893700299?pr=2591">Build and test / lts (17, macos-latest)</a>)</summary>
```
java.lang.AssertionError: expected 2 or 3 ticks (got 4). Expression: (n in [2, 3]). Values: n = 4
at org.codehaus.groovy.runtime.InvokerHelper.createAssertError(InvokerHelper.java:402)
at TestScript20.run(TestScript20.groovy:28)
at groovy.lang.GroovyShell.evaluate(GroovyShell.java:553)
at groovy.lang.GroovyShell.evaluate(GroovyShell.java:588)
at groovy.lang.GroovyShell.evaluate(GroovyShell.java:572)
at groovy.test.GroovyAssert.assertScript(GroovyAssert.java:106)
at groovy.test.GroovyAssert.assertScript(GroovyAssert.java:96)
at groovy.concurrent.ActorTest.testScheduleAtFixedRateCancelStopsFurtherFires(ActorTest.groovy:1372)
```
</details>
### Muted Tests
> [!NOTE]
> Checks are currently running using the configuration below.
Select tests to mute in this pull request:
🔲 ActorTest > testScheduleAtFixedRateCancelStopsFurtherFires\(\) <!-- uniqueId=[engine:junit-jupiter]/[class:groovy.concurrent.ActorTest]/[method:testScheduleAtFixedRateCancelStopsFurtherFires()] -->
Reuse successful test results:
🔲 ♻️ Only rerun the tests that failed or were muted before
Click the checkbox to trigger a rerun:
🔲 **Rerun jobs**
---
_Learn more about TestLens at [testlens.app](https://testlens.app)._
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@groovy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GROOVY-12065: Implement peephole optimization for bytecode generation [groovy]
Posted by "testlens-app[bot] (via GitHub)" <gi...@apache.org>.
testlens-app[bot] commented on PR #2591:
URL: https://github.com/apache/groovy/pull/2591#issuecomment-4640065265
## 🚨 TestLens detected 1 failed test 🚨
Here is what you can do:
1) Inspect the test failures carefully.
2) If you are convinced that some of the tests are flaky, you can mute them below.
3) Finally, trigger a rerun by checking the rerun checkbox.
### Test Summary
| Check | Project/Task | Test | Runs |
|---|---|---|---|
| [Build and test / lts \(17, macos-latest\)](https://github.com/apache/groovy/actions/runs/27070903879/job/79899860733?pr=2591) | :test | ActorTest > testScheduleAtFixedRateCancelStopsFurtherFires\(\) | ❌ |
🏷️ Commit: 2a55a905fbd135cfa4b0415fe659853691457cb2
▶️ Tests: 84666 executed
🟡 Checks: 8/56 completed
### Test Failures
<details>
<summary><strong>ActorTest > testScheduleAtFixedRateCancelStopsFurtherFires()</strong> (:test in <a href="https://github.com/apache/groovy/actions/runs/27070903879/job/79899860733?pr=2591">Build and test / lts (17, macos-latest)</a>)</summary>
```
java.lang.AssertionError: expected 2 or 3 ticks (got 5). Expression: (n in [2, 3]). Values: n = 5
at org.codehaus.groovy.runtime.InvokerHelper.createAssertError(InvokerHelper.java:402)
at TestScript20.run(TestScript20.groovy:28)
at groovy.lang.GroovyShell.evaluate(GroovyShell.java:553)
at groovy.lang.GroovyShell.evaluate(GroovyShell.java:588)
at groovy.lang.GroovyShell.evaluate(GroovyShell.java:572)
at groovy.test.GroovyAssert.assertScript(GroovyAssert.java:106)
at groovy.test.GroovyAssert.assertScript(GroovyAssert.java:96)
at groovy.concurrent.ActorTest.testScheduleAtFixedRateCancelStopsFurtherFires(ActorTest.groovy:1372)
```
</details>
### Muted Tests
> [!NOTE]
> Checks are currently running using the configuration below.
Select tests to mute in this pull request:
🔲 ActorTest > testScheduleAtFixedRateCancelStopsFurtherFires\(\) <!-- uniqueId=[engine:junit-jupiter]/[class:groovy.concurrent.ActorTest]/[method:testScheduleAtFixedRateCancelStopsFurtherFires()] -->
Reuse successful test results:
🔲 ♻️ Only rerun the tests that failed or were muted before
Click the checkbox to trigger a rerun:
🔲 **Rerun jobs**
---
_Learn more about TestLens at [testlens.app](https://testlens.app)._
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@groovy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GROOVY-12065: Implement peephole optimization for constant loading in bytecode generation [groovy]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #2591:
URL: https://github.com/apache/groovy/pull/2591#issuecomment-4639290028
### JMH summary — indy (commit `94f84ea`)
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 | 0.957 × | 26 |
| core | 0.978 × | 77 |
| grails | 0.955 × | 80 |
<sub>Baseline: <code>dev/bench/jmh/<part>/indy/data.js</code> on gh-pages, trailing 90 days. <a href="https://apache.github.io/groovy/dev/bench/jmh/summary.html">Daily dashboard</a> · <a href="https://apache.github.io/groovy/dev/bench/jmh/">Per-suite raw data</a></sub>
<!-- jmh-summary:indy -->
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@groovy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GROOVY-12065: Implement peephole optimization for bytecode generation [groovy]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #2591:
URL: https://github.com/apache/groovy/pull/2591#issuecomment-4640122079
## [](https://sonarcloud.io/dashboard?id=apache_groovy&pullRequest=2591) **Quality Gate passed**
Issues
 [4 New issues](https://sonarcloud.io/project/issues?id=apache_groovy&pullRequest=2591&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_groovy&pullRequest=2591&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_groovy&pullRequest=2591&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [89.3% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_groovy&pullRequest=2591&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_groovy&pullRequest=2591&metric=new_duplicated_lines_density&view=list)
<!-- sqra-placement-anchor -->
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_groovy&pullRequest=2591)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@groovy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GROOVY-12065: Implement peephole optimization for bytecode generation [groovy]
Posted by "eric-milles (via GitHub)" <gi...@apache.org>.
eric-milles commented on code in PR #2591:
URL: https://github.com/apache/groovy/pull/2591#discussion_r3369516921
##########
src/main/java/org/codehaus/groovy/classgen/asm/OperandStack.java:
##########
@@ -585,42 +571,11 @@ private static void pushPrimitiveConstant(final MethodVisitor mv, final Object v
boolean isChar = isPrimitiveChar(type);
if (isInt || isShort || isByte || isChar) {
int val = isInt ? (Integer) value : isShort ? (Short) value : isChar ? (Character) value : (Byte) value;
- BytecodeHelper.pushConstant(mv, val);
- } else if (isPrimitiveLong(type)) {
- if ((Long) value == 0L) {
- mv.visitInsn(LCONST_0);
- } else if ((Long) value == 1L) {
- mv.visitInsn(LCONST_1);
- } else {
- mv.visitLdcInsn(value);
- }
- } else if (isPrimitiveFloat(type)) {
- // GROOVY-9797: Use Float.equals to differentiate between positive and negative zero
- if (value.equals(0f)) {
- mv.visitInsn(FCONST_0);
- } else if ((Float) value == 1f) {
- mv.visitInsn(FCONST_1);
- } else if ((Float) value == 2f) {
- mv.visitInsn(FCONST_2);
- } else {
- mv.visitLdcInsn(value);
- }
- } else if (isPrimitiveDouble(type)) {
- // GROOVY-9797: Use Double.equals to differentiate between positive and negative zero
- if (value.equals(0d)) {
- mv.visitInsn(DCONST_0);
- } else if ((Double) value == 1d) {
- mv.visitInsn(DCONST_1);
- } else {
- mv.visitLdcInsn(value);
- }
+ mv.visitLdcInsn(val);
+ } else if (isPrimitiveLong(type) || isPrimitiveFloat(type) || isPrimitiveDouble(type)) {
+ mv.visitLdcInsn(value);
Review Comment:
Doesn't this require LCONST_0 and so on so the operand stack type is correct?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@groovy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GROOVY-12065: Implement peephole optimization for bytecode generation [groovy]
Posted by "blackdrag (via GitHub)" <gi...@apache.org>.
blackdrag commented on code in PR #2591:
URL: https://github.com/apache/groovy/pull/2591#discussion_r3384712327
##########
src/main/java/org/codehaus/groovy/classgen/asm/OperandStack.java:
##########
@@ -585,42 +571,11 @@ private static void pushPrimitiveConstant(final MethodVisitor mv, final Object v
boolean isChar = isPrimitiveChar(type);
if (isInt || isShort || isByte || isChar) {
int val = isInt ? (Integer) value : isShort ? (Short) value : isChar ? (Character) value : (Byte) value;
- BytecodeHelper.pushConstant(mv, val);
- } else if (isPrimitiveLong(type)) {
- if ((Long) value == 0L) {
- mv.visitInsn(LCONST_0);
- } else if ((Long) value == 1L) {
- mv.visitInsn(LCONST_1);
- } else {
- mv.visitLdcInsn(value);
- }
- } else if (isPrimitiveFloat(type)) {
- // GROOVY-9797: Use Float.equals to differentiate between positive and negative zero
- if (value.equals(0f)) {
- mv.visitInsn(FCONST_0);
- } else if ((Float) value == 1f) {
- mv.visitInsn(FCONST_1);
- } else if ((Float) value == 2f) {
- mv.visitInsn(FCONST_2);
- } else {
- mv.visitLdcInsn(value);
- }
- } else if (isPrimitiveDouble(type)) {
- // GROOVY-9797: Use Double.equals to differentiate between positive and negative zero
- if (value.equals(0d)) {
- mv.visitInsn(DCONST_0);
- } else if ((Double) value == 1d) {
- mv.visitInsn(DCONST_1);
- } else {
- mv.visitLdcInsn(value);
- }
+ mv.visitLdcInsn(val);
+ } else if (isPrimitiveLong(type) || isPrimitiveFloat(type) || isPrimitiveDouble(type)) {
+ mv.visitLdcInsn(value);
Review Comment:
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
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@groovy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GROOVY-12065: Implement peephole optimization for bytecode generation [groovy]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #2591:
URL: https://github.com/apache/groovy/pull/2591#issuecomment-4879852627
## [](https://sonarcloud.io/dashboard?id=apache_groovy&pullRequest=2591) **Quality Gate passed**
Issues
 [4 New issues](https://sonarcloud.io/project/issues?id=apache_groovy&pullRequest=2591&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_groovy&pullRequest=2591&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_groovy&pullRequest=2591&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [94.6% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_groovy&pullRequest=2591&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_groovy&pullRequest=2591&metric=new_duplicated_lines_density&view=list)
<!-- sqra-placement-anchor -->
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_groovy&pullRequest=2591)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@groovy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GROOVY-12065: Implement peephole optimization for bytecode generation [groovy]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #2591:
URL: https://github.com/apache/groovy/pull/2591#issuecomment-4881071174
## [](https://sonarcloud.io/dashboard?id=apache_groovy&pullRequest=2591) **Quality Gate passed**
Issues
 [4 New issues](https://sonarcloud.io/project/issues?id=apache_groovy&pullRequest=2591&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_groovy&pullRequest=2591&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_groovy&pullRequest=2591&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [94.6% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_groovy&pullRequest=2591&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_groovy&pullRequest=2591&metric=new_duplicated_lines_density&view=list)
<!-- sqra-placement-anchor -->
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_groovy&pullRequest=2591)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@groovy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GROOVY-12065: Implement peephole optimization for constant loading in bytecode generation [groovy]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #2591:
URL: https://github.com/apache/groovy/pull/2591#issuecomment-4639240554
## [Codecov](https://app.codecov.io/gh/apache/groovy/pull/2591?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:x: Patch coverage is `93.02326%` with `6 lines` in your changes missing coverage. Please review.
:white_check_mark: Project coverage is 68.3123%. Comparing base ([`c92ba5f`](https://app.codecov.io/gh/apache/groovy/commit/c92ba5fc82d411849e2b621df0ed43baca712433?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`043340d`](https://app.codecov.io/gh/apache/groovy/commit/043340d7bff41bda81fa8a80c1c97597a3486eb7?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
| [Files with missing lines](https://app.codecov.io/gh/apache/groovy/pull/2591?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
|---|---|---|
| [.../classgen/asm/PeepholeOptimizingMethodVisitor.java](https://app.codecov.io/gh/apache/groovy/pull/2591?src=pr&el=tree&filepath=src%2Fmain%2Fjava%2Forg%2Fcodehaus%2Fgroovy%2Fclassgen%2Fasm%2FPeepholeOptimizingMethodVisitor.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3JjL21haW4vamF2YS9vcmcvY29kZWhhdXMvZ3Jvb3Z5L2NsYXNzZ2VuL2FzbS9QZWVwaG9sZU9wdGltaXppbmdNZXRob2RWaXNpdG9yLmphdmE=) | 95.6522% | [2 Missing and 1 partial :warning: ](https://app.codecov.io/gh/apache/groovy/pull/2591?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [...rg/codehaus/groovy/classgen/AsmClassGenerator.java](https://app.codecov.io/gh/apache/groovy/pull/2591?src=pr&el=tree&filepath=src%2Fmain%2Fjava%2Forg%2Fcodehaus%2Fgroovy%2Fclassgen%2FAsmClassGenerator.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3JjL21haW4vamF2YS9vcmcvY29kZWhhdXMvZ3Jvb3Z5L2NsYXNzZ2VuL0FzbUNsYXNzR2VuZXJhdG9yLmphdmE=) | 77.7778% | [2 Missing :warning: ](https://app.codecov.io/gh/apache/groovy/pull/2591?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [...org/codehaus/groovy/classgen/asm/OperandStack.java](https://app.codecov.io/gh/apache/groovy/pull/2591?src=pr&el=tree&filepath=src%2Fmain%2Fjava%2Forg%2Fcodehaus%2Fgroovy%2Fclassgen%2Fasm%2FOperandStack.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3JjL21haW4vamF2YS9vcmcvY29kZWhhdXMvZ3Jvb3Z5L2NsYXNzZ2VuL2FzbS9PcGVyYW5kU3RhY2suamF2YQ==) | 83.3333% | [1 Missing :warning: ](https://app.codecov.io/gh/apache/groovy/pull/2591?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
<details><summary>Additional details and impacted files</summary>
[](https://app.codecov.io/gh/apache/groovy/pull/2591?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
```diff
@@ Coverage Diff @@
## master #2591 +/- ##
==================================================
+ Coverage 68.3072% 68.3123% +0.0051%
- Complexity 33391 33414 +23
==================================================
Files 1518 1519 +1
Lines 126871 126920 +49
Branches 23000 23003 +3
==================================================
+ Hits 86662 86702 +40
- Misses 32526 32531 +5
- Partials 7683 7687 +4
```
| [Files with missing lines](https://app.codecov.io/gh/apache/groovy/pull/2591?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [...g/codehaus/groovy/classgen/asm/BytecodeHelper.java](https://app.codecov.io/gh/apache/groovy/pull/2591?src=pr&el=tree&filepath=src%2Fmain%2Fjava%2Forg%2Fcodehaus%2Fgroovy%2Fclassgen%2Fasm%2FBytecodeHelper.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3JjL21haW4vamF2YS9vcmcvY29kZWhhdXMvZ3Jvb3Z5L2NsYXNzZ2VuL2FzbS9CeXRlY29kZUhlbHBlci5qYXZh) | `87.1935% <100.0000%> (-0.7518%)` | :arrow_down: |
| [...org/codehaus/groovy/classgen/asm/OperandStack.java](https://app.codecov.io/gh/apache/groovy/pull/2591?src=pr&el=tree&filepath=src%2Fmain%2Fjava%2Forg%2Fcodehaus%2Fgroovy%2Fclassgen%2Fasm%2FOperandStack.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3JjL21haW4vamF2YS9vcmcvY29kZWhhdXMvZ3Jvb3Z5L2NsYXNzZ2VuL2FzbS9PcGVyYW5kU3RhY2suamF2YQ==) | `77.6398% <83.3333%> (-0.9730%)` | :arrow_down: |
| [...rg/codehaus/groovy/classgen/AsmClassGenerator.java](https://app.codecov.io/gh/apache/groovy/pull/2591?src=pr&el=tree&filepath=src%2Fmain%2Fjava%2Forg%2Fcodehaus%2Fgroovy%2Fclassgen%2FAsmClassGenerator.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3JjL21haW4vamF2YS9vcmcvY29kZWhhdXMvZ3Jvb3Z5L2NsYXNzZ2VuL0FzbUNsYXNzR2VuZXJhdG9yLmphdmE=) | `85.1744% <77.7778%> (+0.0216%)` | :arrow_up: |
| [.../classgen/asm/PeepholeOptimizingMethodVisitor.java](https://app.codecov.io/gh/apache/groovy/pull/2591?src=pr&el=tree&filepath=src%2Fmain%2Fjava%2Forg%2Fcodehaus%2Fgroovy%2Fclassgen%2Fasm%2FPeepholeOptimizingMethodVisitor.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3JjL21haW4vamF2YS9vcmcvY29kZWhhdXMvZ3Jvb3Z5L2NsYXNzZ2VuL2FzbS9QZWVwaG9sZU9wdGltaXppbmdNZXRob2RWaXNpdG9yLmphdmE=) | `95.6522% <95.6522%> (ø)` | |
... and [3 files with indirect coverage changes](https://app.codecov.io/gh/apache/groovy/pull/2591/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
</details>
<details><summary> :rocket: New features to boost your workflow: </summary>
- :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@groovy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GROOVY-12065: Implement peephole optimization for bytecode generation [groovy]
Posted by "testlens-app[bot] (via GitHub)" <gi...@apache.org>.
testlens-app[bot] commented on PR #2591:
URL: https://github.com/apache/groovy/pull/2591#issuecomment-4639688743
## 🚨 TestLens detected 1 failed test 🚨
Here is what you can do:
1) Inspect the test failures carefully.
2) If you are convinced that some of the tests are flaky, you can mute them below.
3) Finally, trigger a rerun by checking the rerun checkbox.
### Test Summary
| Check | Project/Task | Test | Runs |
|---|---|---|---|
| [Build and test / lts \(17, windows-latest, 1\)](https://github.com/apache/groovy/actions/runs/27067179914/job/79889976995?pr=2591) | :test | GenericsSTCTest > testMockito\(\) | ❌ |
🏷️ Commit: d4cb73bf05d4edb8ba18f4d815876bb5d45ae3b0
▶️ Tests: 176508 executed
🟡 Checks: 40/56 completed
### Test Failures
<details>
<summary><strong>GenericsSTCTest > testMockito()</strong> (:test in <a href="https://github.com/apache/groovy/actions/runs/27067179914/job/79889976995?pr=2591">Build and test / lts (17, windows-latest, 1)</a>)</summary>
```
org.codehaus.groovy.control.MultipleCompilationErrorsException: startup failed:
TestScript9823666d411048d19d47efeafdbf4bdc.groovy: 3: unable to resolve class org.mockito.Mockito
@ line 3, column 27.
import static org.mockito.Mockito.*
^
1 error
at org.codehaus.groovy.control.ErrorCollector.failIfErrors(ErrorCollector.java:333)
at org.codehaus.groovy.control.CompilationUnit$ISourceUnitOperation.doPhaseOperation(CompilationUnit.java:989)
at org.codehaus.groovy.control.CompilationUnit.processPhaseOperations(CompilationUnit.java:749)
at org.codehaus.groovy.control.CompilationUnit.compile(CompilationUnit.java:723)
at groovy.lang.GroovyClassLoader.doParseClass(GroovyClassLoader.java:423)
at groovy.lang.GroovyClassLoader.lambda$parseClass$3(GroovyClassLoader.java:364)
at org.codehaus.groovy.runtime.memoize.ConcurrentCommonCache.getAndPut(ConcurrentCommonCache.java:143)
at groovy.lang.GroovyClassLoader.parseClass(GroovyClassLoader.java:364)
at groovy.lang.GroovyShell.parseClass(GroovyShell.java:665)
at groovy.lang.GroovyShell.parse(GroovyShell.java:678)
at groovy.lang.GroovyShell.parse(GroovyShell.java:690)
at groovy.lang.GroovyShell.evaluate(GroovyShell.java:552)
at groovy.lang.GroovyShell.evaluate(GroovyShell.java:588)
at groovy.lang.GroovyShell.evaluate(GroovyShell.java:572)
at groovy.transform.stc.StaticTypeCheckingTestCase.assertScript(StaticTypeCheckingTestCase.groovy:78)
at groovy.transform.stc.GenericsSTCTest.testMockito(GenericsSTCTest.groovy:5962)
```
</details>
### Muted Tests
> [!NOTE]
> Checks are currently running using the configuration below.
Select tests to mute in this pull request:
🔲 GenericsSTCTest > testMockito\(\) <!-- uniqueId=[engine:junit-jupiter]/[class:groovy.transform.stc.GenericsSTCTest]/[method:testMockito()] -->
Reuse successful test results:
🔲 ♻️ Only rerun the tests that failed or were muted before
Click the checkbox to trigger a rerun:
🔲 **Rerun jobs**
---
_Learn more about TestLens at [testlens.app](https://testlens.app)._
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@groovy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GROOVY-12065: Implement peephole optimization for bytecode generation [groovy]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #2591:
URL: https://github.com/apache/groovy/pull/2591#issuecomment-4639867095
## [](https://sonarcloud.io/dashboard?id=apache_groovy&pullRequest=2591) **Quality Gate passed**
Issues
 [4 New issues](https://sonarcloud.io/project/issues?id=apache_groovy&pullRequest=2591&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_groovy&pullRequest=2591&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_groovy&pullRequest=2591&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [89.2% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_groovy&pullRequest=2591&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_groovy&pullRequest=2591&metric=new_duplicated_lines_density&view=list)
<!-- sqra-placement-anchor -->
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_groovy&pullRequest=2591)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@groovy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org