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/`.*
   


-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to