[
https://issues.apache.org/jira/browse/GROOVY-12065?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18086599#comment-18086599
]
ASF GitHub Bot commented on GROOVY-12065:
-----------------------------------------
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>
> Implement peephole optimization for constant loading in bytecode generation
> ---------------------------------------------------------------------------
>
> Key: GROOVY-12065
> URL: https://issues.apache.org/jira/browse/GROOVY-12065
> Project: Groovy
> Issue Type: Improvement
> Reporter: Daniel Sun
> Priority: Major
>
> h3. Background
> Groovy's bytecode generator relied on ad-hoc, per-type conditional branches
> scattered across {{OperandStack}} and {{BytecodeHelper}} to select compact
> constant-loading instructions ({{{}ICONST_x{}}}, {{{}LCONST_x{}}},
> {{{}FCONST_x{}}}, {{{}DCONST_x{}}}, {{{}BIPUSH{}}}, {{{}SIPUSH{}}}) over the
> generic {{LDC}} instruction. This logic was fragile, inconsistent, and
> incomplete — for instance, {{BytecodeHelper.pushConstant()}} was missing the
> {{ICONST_M1}} case for the integer constant {{{}-1{}}}.
> h3. Change
> A new {{PeepholeOptimizingMethodVisitor}} — a {{MethodVisitor}} decorator —
> is introduced and inserted into the per-method visitor chain in
> {{{}AsmClassGenerator{}}}. It intercepts every {{visitLdcInsn}} and
> {{visitIntInsn(BIPUSH/SIPUSH, ...)}} call and rewrites each to the shortest
> valid equivalent opcode:
> ||Value||Emitted instruction||
> |{{int}} −1|{{ICONST_M1}}|
> |{{int}} 0–5|{{ICONST_0}} – {{ICONST_5}}|
> |{{int}} in [−128, 127]|{{BIPUSH}}|
> |{{int}} in [−32768, 32767]|{{SIPUSH}}|
> |{{long}} 0 / 1|{{LCONST_0}} / {{LCONST_1}}|
> |{{float}} 0 / 1 / 2|{{FCONST_0}} / {{FCONST_1}} / {{FCONST_2}}|
> |{{double}} 0 / 1|{{DCONST_0}} / {{DCONST_1}}|
> Signed-zero {{float}} and {{double}} values are handled correctly by
> comparing raw bit patterns (via {{Float.floatToRawIntBits}} /
> {{{}Double.doubleToRawLongBits{}}}) rather than {{{}=={}}}, preserving the
> {{-0.0}} / {{+0.0}} distinction that a straight equality check would collapse.
> All per-type branches previously inlined in {{OperandStack}} are replaced
> with plain {{visitLdcInsn(value)}} calls; the peephole visitor centralizes
> the selection uniformly for every generated method.
> {{BytecodeHelper.pushConstant()}} also gains the previously missing
> {{ICONST_M1}} branch.
>
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
