[
https://issues.apache.org/jira/browse/GROOVY-12284?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18106841#comment-18106841
]
ASF GitHub Bot commented on GROOVY-12284:
-----------------------------------------
daniellansun commented on PR #2822:
URL: https://github.com/apache/groovy/pull/2822#issuecomment-5377696003
# GROOVY-12284 Performance Verification Report
**Specialize indy `sameClasses` guards for arity 1–4**
(after PR #2822 review follow-up: `insertArguments` + `changeReturnType`)
| Item | Value |
|---|---|
| Issue | [GROOVY-12284](https://issues.apache.org/jira/browse/GROOVY-12284)
|
| PR | [#2822](https://github.com/apache/groovy/pull/2822) |
| Tree under test | GROOVY-12284 working tree (`47bd7b07` + review
follow-up: `insertArguments`, `changeReturnType(boolean.class)`, no `n==0`
combinator) |
| Baseline | `e801df39580ab480249e561e90cf661ce86917f5` |
| Date | 2026-08-22 |
| Core tests | `./gradlew :test` — **16949 passed / 60 skipped / 0 failed** |
| Verdict | **PASS** — specialised guards remain 1.5–1.9× (arity 1–3) and
**4.68×** (arity 4) vs the collector parent; `insertArguments` vs chained
`bindTo` is **0.99×** (noise); arity 5 / `@CompileStatic` stay at parity |
---
## 1. Executive summary
The live call-site helper now binds expected classes with **one**
`MethodHandles.insertArguments` against `SAME_CLASS_GUARDS[n]`, and takes
the invocation `MethodType` so the guard signature is
`callType.changeReturnType(boolean.class)`. That is the shape Jochen and
Paul asked for on #2822. It does not change the user-visible mechanism:
arity 1–4 avoid `asCollector`; arity ≥ 5 still collects.
Order-balanced A/B against `e801df39` (same host, JDK, JMH annotations as
the pre-review run):
| Row | Geomean HEAD / parent |
|---|---|
| `dynamic_arity1` | **1.54×** |
| `dynamic_arity2` | **1.86×** |
| `dynamic_arity3` | **1.93×** |
| `dynamic_arity4` | **4.68×** |
| `dynamic_arity5` (collector both sides) | 1.11× |
| `cs_arity2` / `cs_arity5` | **1.01× / 0.96×** |
| Isolated combinator, insert vs collector (arity 1 / 2 / 4) | 1.15× / 1.19×
/ 1.29× |
| Isolated combinator, insert vs chained `bindTo` (arity 4) | **0.99×** |
| `cpuIntegerOps` ruler | **1.00×** |
GC fingerprint is unchanged: parent arity 4 = **32 B/op**, HEAD ≈ 0;
arity 5 = **40 B/op** on both sides.
`insertArguments` is the right production shape (one adapter, same JIT
result as four `bindTo`s). The end-to-end win is the specialised leaf
versus `asCollector`, not bind-vs-insert.
**Performance verification: PASS.**
---
## 2. What changed since `47bd7b07`
Review consensus (Jochen Theodorou, Paul King):
1. Table of specialised handles + **one** `insertArguments` (not chained
`bindTo`).
2. Guard signature from `handle.type().changeReturnType(boolean.class)`.
3. Drop the unreachable `n == 0` constant-`true` combinator.
4. Fix MH-bench javadoc (`@Setup` fails pre-12284; the class still compiles).
5. Keep the JMH benches.
Out of scope, as Jochen marked it: `ClassValue` cache of forms by call-site
index.
---
## 3. Methodology
| Item | Value |
|---|---|
| OS / host | Linux 6.15.5, `hera` |
| CPU | AMD EPYC 7763, 6 vCPUs (KVM) |
| JDK | Amazon Corretto **25.0.2** |
| JMH | 1.37; `@Warmup(3×2s) @Measurement(5×2s) @Fork(2)` |
| Parent | worktree `/tmp/groovy-12284-parent` @ `e801df39` |
| Bench parity | `SameClassesGuardBench.groovy` identical in both trees |
| Order | T1 HEAD then parent; T2 parent then HEAD |
| Suites | indy A/B, GC profiler, `SameClassesGuardMhBench` (HEAD), CPU
rulers |
Throughput ratio = HEAD / parent. Order-balanced result = geomean of the
two trial ratios. Raw JSON: `/tmp/groovy-12284-perf2/`.
---
## 4. Results
### 4.1 End-to-end indy (ops/ms)
| Benchmark | T1 parent | T1 HEAD | T1 | T2 parent | T2 HEAD | T2 | Geomean |
|---|---:|---:|---:|---:|---:|---:|---:|
| `dynamic_arity1` | 659 888 ± 26 847 | 1 063 775 ± 65 319 | 1.612× | 658
284 ± 25 333 | 964 529 ± 88 298 | 1.465× | **1.54×** |
| `dynamic_arity2` | 496 882 ± 18 108 | 959 441 ± 37 131 | 1.931× | 489 092
± 27 441 | 871 731 ± 140 016 | 1.782× | **1.86×** |
| `dynamic_arity3` | 405 117 ± 10 264 | 759 771 ± 81 255 | 1.875× | 402 888
± 25 325 | 796 490 ± 31 832 | 1.977× | **1.93×** |
| `dynamic_arity4` | 130 981 ± 19 018 | 629 695 ± 22 114 | 4.808× | 137 525
± 3 348 | 625 549 ± 18 672 | 4.549× | **4.68×** |
| `dynamic_arity5` | 106 107 ± 3 115 | 114 814 ± 13 727 | 1.082× | 102 935 ±
9 850 | 117 950 ± 7 992 | 1.146× | 1.11× |
| `cs_arity2` | 1 570 710 ± 29 917 | 1 610 876 ± 47 158 | 1.026× | 1 578 283
± 89 044 | 1 563 538 ± 70 906 | 0.991× | **1.01×** |
| `cs_arity5` | 1 593 835 ± 73 043 | 1 538 583 ± 166 577 | 0.965× | 1 609
468 ± 30 792 | 1 550 691 ± 63 009 | 0.963× | 0.96× |
Treatment geomean arity 1–4: **2.25×**. Arity 1–3 only: **1.76×**.
CS is clean on both trials (no dirty fork this run).
### 4.2 Allocation (`gc.alloc.rate.norm`)
| Benchmark | Parent B/op | HEAD B/op |
|---|---:|---:|
| `cs_*` / `dynamic_arity1–3` | ≈ 0 | ≈ 0 |
| `dynamic_arity4` | **32** | ≈ 0 |
| `dynamic_arity5` | **40** | **40** |
Same structural fingerprint as before the review follow-up.
`Object[4]` = 32 B, `Object[5]` = 40 B (compressed oops). C2 still
scalar-replaces `n ≤ 3`.
### 4.3 Isolated combinator (intra-HEAD)
| Row | ops/ms | vs collector | vs chained `bindTo` |
|---|---:|---:|---:|
| `collector_arity1` | 173 850 ± 8 694 | — | — |
| `specialised_arity1` (`insertArguments`) | 199 279 ± 5 964 | **1.15×** | —
|
| `collector_arity2` | 157 001 ± 6 634 | — | — |
| `specialised_arity2` | 187 601 ± 4 442 | **1.19×** | — |
| `collector_arity4` | 114 896 ± 2 409 | — | — |
| `specialised_arity4` (`insertArguments`) | 147 758 ± 18 155 | **1.29×** |
**0.99×** |
| `specialised_bindTo_arity4` | 149 060 ± 23 010 | 1.30× | — |
Jochen’s hypothesis that one `insertArguments` might beat four `bindTo`s
does not show up as a throughput delta on this JDK (0.99×, CIs overlap).
It is still the better code: one adapter, same JIT result, matches
`IndyCompoundAssign`.
### 4.4 Calibration
| Ruler | Parent | HEAD | Speedup (P/H) |
|---|---:|---:|---:|
| `cpuIntegerOps` | 413.2 ± 4.0 µs | 412.6 ± 12.0 µs | **1.00×** |
| `memoryPointerChase` | 1557 ± 76 µs | 1534 ± 123 µs | **1.01×** |
| `allocationChurn` | 91.3 ± 8.6 µs | 183 ± 470 µs | unusable (HEAD CI >
mean) |
Hardware did not drift. Ignore HEAD `allocationChurn` as before.
---
## 5. Conclusions
The review follow-up does **not** give back the GROOVY-12284 win:
- Linked monomorphic arity 1–3: **1.5–1.9×** (combinator / inlining).
- Arity 4: **4.68×** and 32 B/op → 0.
- Arity 5 and `@CompileStatic`: parity.
- `insertArguments` vs chained `bindTo`: **0.99×** isolated; ship
`insertArguments`.
**Performance verification: PASS.**
Keep `SameClassesGuardBench.dynamic_arity4` / `dynamic_arity5` under GC
profiling in the indy regression rotation.
### Reproduction
```bash
./gradlew :test --tests
org.codehaus.groovy.vmplugin.v8.IndySameClassesGuardTest
./gradlew :test
./gradlew :perf:jmh -PbenchInclude=SameClassesGuardBench
-PjmhResultFormat=JSON
./gradlew :perf:jmh -PbenchInclude=SameClassesGuardBench -PjmhProfilers=gc
-PjmhResultFormat=JSON
./gradlew :perf:jmh -PbenchInclude=SameClassesGuardMhBench
-PjmhResultFormat=JSON
```
Parent worktree: `/tmp/groovy-12284-parent` @ `e801df39`.
Runner: `/tmp/groovy-12284-perf2/run.sh`.
---
*Report generated from local JMH A/B measurements on 2026-08-22 after the PR
#2822 review follow-up. Raw JSON: `/tmp/groovy-12284-perf2/`.*
> Specialize indy sameClasses guards for arity 1-4
> ------------------------------------------------
>
> Key: GROOVY-12284
> URL: https://issues.apache.org/jira/browse/GROOVY-12284
> Project: Groovy
> Issue Type: Improvement
> Reporter: Daniel Sun
> Priority: Major
>
> h3. Problem
> When an invokedynamic site is linked with all arguments non-null and at least
> one parameter type that is non-final (or a primitive wrapper — GROOVY-11782),
> {{Selector}} installs a same-class guard:
> {code:java}
> SAME_CLASSES
> .bindTo(expectedClasses)
> .asCollector(Object[].class, n)
> .asType(MethodType.methodType(boolean.class, pt));
> {code}
> {{asCollector}} of an {{Object}} array of length {{n}} allocates a fresh
> array on *every later invocation* of that site; the array overload of
> {{sameClasses}} then walks it.
> That is the hot path for ordinary dynamic Groovy calls of the shapes
> {{recv.foo()}}, {{recv.foo(a)}}, {{recv.foo(a, b)}}, {{recv.foo(a, b, c)}} —
> arity 1-4 (receiver plus 0-3 arguments). Dynamic indy sites almost always
> have {{Object}} parameter types, so this guard is the common case, not a rare
> fallback.
> (If any argument is {{null}} at link time, {{Selector}} already installs
> per-slot {{SAME_CLASS}} / {{IS_NULL}} tests and does not use the collector.)
> The classic MOP already specializes this check:
> {{MetaClassHelper.sameClasses}} has overloads for 0-4 arguments so the
> call-site cache does not box arguments into an array. The indy guard did not.
> h3. Goal
> Keep the same guard semantics (return {{false}} if any argument is {{null}}
> or has a different runtime class) without allocating an {{Object}} array on
> the common 1-4 arity shapes.
> h3. Approach
> ||Arity (incl. receiver)||Guard||
> |0|constant {{true}}|
> |1|existing {{SAME_CLASS}}|
> |2|new {{SAME_CLASSES_2}}|
> |3|new {{SAME_CLASSES_3}}|
> |4|new {{SAME_CLASSES_4}}|
> |5 or more|existing {{SAME_CLASSES}} plus {{asCollector}} (unchanged)|
> Expected classes are bound with {{bindTo}}. One {{guardWithTest}} at the
> site, via a single {{Selector.sameClassesGuard(args, pt)}} helper.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
