[
https://issues.apache.org/jira/browse/GROOVY-12164?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18096102#comment-18096102
]
ASF GitHub Bot commented on GROOVY-12164:
-----------------------------------------
codecov-commenter commented on PR #2710:
URL: https://github.com/apache/groovy/pull/2710#issuecomment-4966318306
##
[Codecov](https://app.codecov.io/gh/apache/groovy/pull/2710?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 `67.64706%` with `11 lines` in your changes missing
coverage. Please review.
:white_check_mark: Project coverage is 69.1066%. Comparing base
([`9b4176b`](https://app.codecov.io/gh/apache/groovy/commit/9b4176bc49be4361134d4ed274bcc6a56145488d?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache))
to head
([`440dc47`](https://app.codecov.io/gh/apache/groovy/commit/440dc4736d15fcd0c5ba206738ae7487e38dfd79?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/2710?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
| Patch % | Lines |
|---|---|---|
|
[src/main/java/groovy/lang/Closure.java](https://app.codecov.io/gh/apache/groovy/pull/2710?src=pr&el=tree&filepath=src%2Fmain%2Fjava%2Fgroovy%2Flang%2FClosure.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3JjL21haW4vamF2YS9ncm9vdnkvbGFuZy9DbG9zdXJlLmphdmE=)
| 67.6471% | [6 Missing and 5 partials :warning:
](https://app.codecov.io/gh/apache/groovy/pull/2710?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/2710?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
```diff
@@ Coverage Diff @@
## master #2710 +/- ##
==================================================
- Coverage 69.1141% 69.1066% -0.0075%
+ Complexity 34250 34248 -2
==================================================
Files 1537 1537
Lines 129373 129403 +30
Branches 23508 23526 +18
==================================================
+ Hits 89415 89426 +11
- Misses 31939 31948 +9
- Partials 8019 8029 +10
```
| [Files with missing
lines](https://app.codecov.io/gh/apache/groovy/pull/2710?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
| Coverage Δ | |
|---|---|---|
|
[src/main/java/groovy/lang/Closure.java](https://app.codecov.io/gh/apache/groovy/pull/2710?src=pr&el=tree&filepath=src%2Fmain%2Fjava%2Fgroovy%2Flang%2FClosure.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3JjL21haW4vamF2YS9ncm9vdnkvbGFuZy9DbG9zdXJlLmphdmE=)
| `77.9935% <67.6471%> (-1.2179%)` | :arrow_down: |
... and [4 files with indirect coverage
changes](https://app.codecov.io/gh/apache/groovy/pull/2710/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>
> Closure.call fast path misses closures with typed parameters (~6x slower via
> full metaclass dispatch)
> -----------------------------------------------------------------------------------------------------
>
> Key: GROOVY-12164
> URL: https://issues.apache.org/jira/browse/GROOVY-12164
> Project: Groovy
> Issue Type: Improvement
> Reporter: Paul King
> Priority: Major
>
> Description:
> The Closure.call(Object...) fast path added in GROOVY-11911 caches a direct
> doCall/call Method per closure subclass (CallOverride, keyed via
> {{type.getMethod("call", Object.class)}}). A generated closure class only
> declares a {{call(Object)}} override when its doCall takes Object — i.e. when
> the closure parameter is untyped ({{it}} or {{ { x -> } }}). A closure with a
> *typed* parameter generates {{doCall(Integer)}}/{{call(Integer)}}, which the
> Object-signature lookup cannot see, so CallOverride resolves to NONE and every
> invocation falls back to full {{getMetaClass().invokeMethod(this, "doCall",
> args)}} dispatch.
> Since callers dispatch through the static type Closure (e.g. every DGM
> iteration method calls {{closure.call(item)}} from Java), the typed
> {{call(Integer)}} overload on the generated class is never selected either.
> Net effect: annotating a closure parameter with its type — normally good
> practice — costs roughly 3-6x in per-element dispatch overhead.
> Measurements (steady-state, median of 9 trials, isolated JVM per variant,
> 12-element List, @CompileStatic enclosing class, JDK 23, master):
> || closure || param || capture || ops/ms ||
> | {{ { x -> s += (int) x } }} (each) | untyped | Reference | 13,408 |
> | {{ { it * it } }} (collect) | untyped | none | 13,173 |
> | {{ { Integer x -> x * x } }} (collect) | typed | none | 4,148 |
> | {{ { Integer x -> s += x } }} (collect) | typed | Reference | 3,606 |
> | {{ { Integer x -> a[0] += x } }} (each) | typed | int[] | 2,471 |
> | {{ { Integer x -> s += x } }} (each) | typed | Reference | 2,229 |
> The typed/untyped split is the dominant factor; capture kind and
> each-vs-collect are second-order.
> Possible fixes (either restores the fast path for typed params):
> 1. Extend CallOverride.lookup to also accept a single-arg typed override
> (resolve the closure's declared one-arg call/doCall whatever its parameter
> type), with an argument compatibility/coercion guard before the cached
> reflective invoke so metaclass coercion semantics (GString->String,
> number conversions, null handling) are preserved, falling back to
> invokeMethod on mismatch.
> 2. Have ClosureWriter always emit a coercing {{call(Object)}} bridge alongside
> the typed doCall (castToType to the declared parameter type, then direct
> doCall), so the existing Object-signature lookup finds every generated
> closure class.
> Option 2 keeps the runtime lookup untouched and localises the semantics in
> generated code, but adds a method per closure class; option 1 is
> runtime-only and also covers pre-existing compiled classes.
> Discovered while benchmarking GROOVY-12151 (closure packing): the packed
> adapter dispatches typed parameters via checkcast/unbox in a generated
> dispatch table and is unaffected, which initially made generated closure
> classes look artificially slow in the typed-parameter comparison.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
