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/07/13 14:49:30 UTC
[PR] GROOVY-12161: Omit identity catch-all for try/catch without finally and slim exception bytecode [groovy]
daniellansun opened a new pull request, #2706:
URL: https://github.com/apache/groovy/pull/2706
https://issues.apache.org/jira/browse/GROOVY-12161
--
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-12161: Omit identity catch-all for try/catch without finally and slim exception bytecode [groovy]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #2706:
URL: https://github.com/apache/groovy/pull/2706#issuecomment-4959673646
### JMH summary — classic (commit `5dfdfba`)
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. The *calibrated*
column divides out this runner's speed vs the baseline hardware, as
measured by Groovy-independent pure-Java ruler benchmarks.
| Group | Speedup | Calibrated | n |
|--------|---------|------------|---|
| bench | 0.969 × | 0.990 × | 84 |
| core | 1.123 × | 0.947 × | 77 |
| grails | 0.970 × | 0.878 × | 80 |
> ⚠️ Runner speed differs ≥15% from the historical baseline hardware for: core-hz. Raw speedups are not meaningful for those parts — use the calibrated column.
<sub>Runner calibration (this run vs baseline hardware): bench 0.98× (26 rulers) · core-ag 0.96× (3 rulers) · core-hz 1.50× (3 rulers) · grails-ad 1.07× (3 rulers) · grails-ez 1.13× (3 rulers)</sub>
<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-12161: Omit identity catch-all for try/catch without finally and slim exception bytecode [groovy]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #2706:
URL: https://github.com/apache/groovy/pull/2706#issuecomment-4959751802
### JMH summary — indy (commit `5dfdfba`)
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. The *calibrated*
column divides out this runner's speed vs the baseline hardware, as
measured by Groovy-independent pure-Java ruler benchmarks.
| Group | Speedup | Calibrated | n |
|--------|---------|------------|---|
| bench | 1.005 × | 1.012 × | 84 |
| core | 3.920 × | 3.309 × | 77 |
| grails | 2.247 × | 2.514 × | 80 |
> ⚠️ Runner speed differs ≥15% from the historical baseline hardware for: core-hz, grails-ez. Raw speedups are not meaningful for those parts — use the calibrated column.
<sub>Runner calibration (this run vs baseline hardware): bench 0.99× (26 rulers) · core-ag 0.98× (3 rulers) · core-hz 1.46× (3 rulers) · grails-ad 0.94× (3 rulers) · grails-ez 0.86× (3 rulers)</sub>
<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-12161: Omit identity catch-all for try/catch without finally and slim exception bytecode [groovy]
Posted by "daniellansun (via GitHub)" <gi...@apache.org>.
daniellansun commented on PR #2706:
URL: https://github.com/apache/groovy/pull/2706#issuecomment-4988154354
If no one objects to this PR, I'll merge it this weekend.
--
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-12161: Omit identity catch-all for try/catch without finally and slim exception bytecode [groovy]
Posted by "testlens-app[bot] (via GitHub)" <gi...@apache.org>.
testlens-app[bot] commented on PR #2706:
URL: https://github.com/apache/groovy/pull/2706#issuecomment-4959753088
## ✅ All tests passed ✅
🏷️ Commit: 47a13fc0cde67baf89ab70d1725d694d85a66c53
▶️ Tests: 103841 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-12161: Omit identity catch-all for try/catch without finally and slim exception bytecode [groovy]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #2706:
URL: https://github.com/apache/groovy/pull/2706#issuecomment-4959611414
## [](https://sonarcloud.io/dashboard?id=apache_groovy&pullRequest=2706) **Quality Gate passed**
Issues
 [2 New issues](https://sonarcloud.io/project/issues?id=apache_groovy&pullRequest=2706&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_groovy&pullRequest=2706&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_groovy&pullRequest=2706&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [100.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_groovy&pullRequest=2706&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_groovy&pullRequest=2706&metric=new_duplicated_lines_density&view=list)
<!-- sqra-placement-anchor -->
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_groovy&pullRequest=2706)
--
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-12161: Omit identity catch-all for try/catch without finally and slim exception bytecode [groovy]
Posted by "paulk-asert (via GitHub)" <gi...@apache.org>.
paulk-asert commented on PR #2706:
URL: https://github.com/apache/groovy/pull/2706#issuecomment-4989351162
Merged. Thanks! Plus I added an additional test.
--
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-12161: Omit identity catch-all for try/catch without finally and slim exception bytecode [groovy]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #2706:
URL: https://github.com/apache/groovy/pull/2706#issuecomment-4959600246
## [Codecov](https://app.codecov.io/gh/apache/groovy/pull/2706?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:white_check_mark: All modified and coverable lines are covered by tests.
:white_check_mark: Project coverage is 69.0906%. Comparing base ([`86c6648`](https://app.codecov.io/gh/apache/groovy/commit/86c6648d1e61436fe513afaa85d8d0c403de3cda?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`47a13fc`](https://app.codecov.io/gh/apache/groovy/commit/47a13fc0cde67baf89ab70d1725d694d85a66c53?dropdown=coverage&el=desc&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/2706?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
```diff
@@ Coverage Diff @@
## master #2706 +/- ##
==================================================
- Coverage 69.1077% 69.0906% -0.0172%
- Complexity 34235 34239 +4
==================================================
Files 1537 1537
Lines 129356 129365 +9
Branches 23503 23509 +6
==================================================
- Hits 89395 89379 -16
- Misses 31941 31962 +21
- Partials 8020 8024 +4
```
| [Files with missing lines](https://app.codecov.io/gh/apache/groovy/pull/2706?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [...org/codehaus/groovy/classgen/asm/CompileStack.java](https://app.codecov.io/gh/apache/groovy/pull/2706?src=pr&el=tree&filepath=src%2Fmain%2Fjava%2Forg%2Fcodehaus%2Fgroovy%2Fclassgen%2Fasm%2FCompileStack.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3JjL21haW4vamF2YS9vcmcvY29kZWhhdXMvZ3Jvb3Z5L2NsYXNzZ2VuL2FzbS9Db21waWxlU3RhY2suamF2YQ==) | `86.3081% <ø> (-0.2773%)` | :arrow_down: |
| [.../codehaus/groovy/classgen/asm/StatementWriter.java](https://app.codecov.io/gh/apache/groovy/pull/2706?src=pr&el=tree&filepath=src%2Fmain%2Fjava%2Forg%2Fcodehaus%2Fgroovy%2Fclassgen%2Fasm%2FStatementWriter.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3JjL21haW4vamF2YS9vcmcvY29kZWhhdXMvZ3Jvb3Z5L2NsYXNzZ2VuL2FzbS9TdGF0ZW1lbnRXcml0ZXIuamF2YQ==) | `96.9697% <100.0000%> (+0.0785%)` | :arrow_up: |
... and [16 files with indirect coverage changes](https://app.codecov.io/gh/apache/groovy/pull/2706/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-12161: Omit identity catch-all for try/catch without finally and slim exception bytecode [groovy]
Posted by "paulk-asert (via GitHub)" <gi...@apache.org>.
paulk-asert merged PR #2706:
URL: https://github.com/apache/groovy/pull/2706
--
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-12161: Omit identity catch-all for try/catch without finally and slim exception bytecode [groovy]
Posted by "blackdrag (via GitHub)" <gi...@apache.org>.
blackdrag commented on code in PR #2706:
URL: https://github.com/apache/groovy/pull/2706#discussion_r3572755044
##########
src/test/groovy/bugs/Groovy11362.groovy:
##########
@@ -33,9 +33,11 @@ final class Groovy11362 extends AbstractBytecodeTestCase {
}
}
'''
+ // Catch parameter must be typed Exception (GROOVY-11362), not Object.
+ // Label numbers are not stable across try/catch codegen improvements.
Review Comment:
They had been stable before, why not anymore?
##########
src/main/java/org/codehaus/groovy/classgen/asm/CompileStack.java:
##########
@@ -941,19 +951,19 @@ private void applyBlockRecorder(final Collection<BlockRecorder> blockRecorders)
if (visitedBlocks.contains(recorder)) continue;
Label end = new Label();
+ // Guarantee a non-empty protected range before excluding finally.
mv.visitInsn(NOP);
mv.visitLabel(end);
-
recorder.closeRange(end);
- // we exclude the finally block from the exception table
- // here to avoid double visiting of finally statements
+ // Exclude the finally body from the exception table so it is not
+ // re-entered if it throws (avoid double application of finally).
recorder.excludedStatement.run();
recorder.startRange(start);
}
- mv.visitInsn(NOP);
+ // start marks the first instruction after all inlined finally blocks.
mv.visitLabel(start);
Review Comment:
I vaguely remember I did need those NOP instructions or else I could not set the label. I think this was for cases in which this was the first instruction in the method... maybe for an empty try-finally? not sure anymore. Or maybe I added it here back then defensively because of a similar problem in combinations with returns? But thinking back I don't think this was because of the bytecode, I think it was because of ASM. So either a bug or me not using it correctly. Very possible this is no longer an issue and thus not required.
--
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-12161: Omit identity catch-all for try/catch without finally and slim exception bytecode [groovy]
Posted by "daniellansun (via GitHub)" <gi...@apache.org>.
daniellansun commented on code in PR #2706:
URL: https://github.com/apache/groovy/pull/2706#discussion_r3575296002
##########
src/test/groovy/bugs/Groovy11362.groovy:
##########
@@ -33,9 +33,11 @@ final class Groovy11362 extends AbstractBytecodeTestCase {
}
}
'''
+ // Catch parameter must be typed Exception (GROOVY-11362), not Object.
+ // Label numbers are not stable across try/catch codegen improvements.
Review Comment:
I hope I have understood what you mean.
In the past years, I find groovy generates more complex bytecode for try-catch statement than Java does, so I wish we could refine them. As we all know, simpler and less bytecode is also friendly to JIT.
See also:
https://issues.apache.org/jira/browse/GROOVY-12161
--
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