[
https://issues.apache.org/jira/browse/GROOVY-12325?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18110675#comment-18110675
]
ASF GitHub Bot commented on GROOVY-12325:
-----------------------------------------
daniellansun commented on PR #2852:
URL: https://github.com/apache/groovy/pull/2852#issuecomment-5510989609
@blackdrag Thanks for the review — the hidden-class encoding staying
acceptable is a relief. The complexity-vs-gain concern is fair, and the benches
were incomplete in the way you described. I have pushed follow-ups and a short
measurement note (`jira-drafts/pr-2852-performance-report.md` locally; I can
paste numbers here).
**Baseline.** `startsWith_java` is now labelled a lower bound only. The
baseline is reflective `CachedMethod.invoke`
(`-Dgroovy.cachedmethod.invoker.disable=true`). Operands are `@Param` fields so
the JIT cannot fold `startsWith` to a constant.
Against `03e303e` on JDK 25 (JMH 1.37, 1 fork, 3×1 s / 5×1 s):
| Path | ns/op |
|---|---|
| `03e303e` `CachedMethod.invoke` | 18.0 ± 2.7 |
| HEAD, generation disabled | 17.8 ± 1.3 |
| HEAD, generated (warmed, `threshold=0`) | 10.1 ± 2.5 |
| Java `startsWith` | 7.3 ± 0.8 |
So the interned-MOP case is about **1.8×** the pre-change path, and the
kill-switch path is not a regression within error.
**Create cost / break-even.** New rows use a fresh `CachedMethod` per
invocation:
- `generate_reflective_fresh` ≈ 0.09 µs
- `generate_then_invoke` (`threshold=0`) ≈ 77 µs
Almost all of that is `defineHiddenClass`. At ~8 ns saved per later invoke,
naïve break-even is on the order of 10⁴ calls **after** generation. That cost
is paid **once per interned `CachedMethod` per VM**, which is the real MOP
(`CachedClass` method table). A throwaway `CachedMethod` used a few hundred
times does not recoup — the `burst_*` rows show that on purpose (250 calls with
default threshold ≈ 104 µs vs 4 µs staying reflective, because the burst
includes one define). Default threshold 100 is unchanged: below
`groovy.indy.optimize.threshold` (1000), so methods that never look hot never
define a class.
The first disable-path run was ~31 ns vs 18 ns on `03e303e` because
`generationAllowed()` re-read system properties on every invoke. That is now
sticky on the instance when generation is off (kill switch / native-image /
Android). The threshold is cached on the instance so lukewarm invokes do not
call `Long.getLong` every time.
**Why `CachedMethod`.** This is not a third invoke engine beside indy and
`CallSiteGenerator`. Those already have a JIT-constant target (linked
`CallSite` / emitted `INVOKE*`). `CachedMethod.invoke` is the leftover
`Method.invoke` on the MOP “Groovy as caller” path (including cold indy still
on `doMethodInvoke`). The trampoline is bound to that `Method` and must not be
fed to indy (wrong `Lookup`). Putting the hook on `CachedMethod` is the
smallest place that sees every such invoke without widening public API. I kept
the fast path to a volatile read + generate-or-reflect; factory/ASM live under
`org.apache.groovy.internal.runtime.invoke`.
Happy to drop the default threshold, or the whole hook behind `disable=true`
by default, if you would rather ship the machinery dark until we have a
longer-lived application bench.
> Speed up CachedMethod.invoke with a generated JIT-constant trampoline
> ---------------------------------------------------------------------
>
> Key: GROOVY-12325
> URL: https://issues.apache.org/jira/browse/GROOVY-12325
> Project: Groovy
> Issue Type: Improvement
> Reporter: Daniel Sun
> Priority: Major
>
> h2. Problem
> {{CachedMethod.invoke}} is the MOP/Java fallback used by {{MetaClassImpl}},
> classic uncompiled call sites, and the default indy cold tier
> ({{invokeColdReflective}} -> {{doMethodInvoke}}).
> That path still calls {{java.lang.reflect.Method.invoke}}. A {{MethodHandle}}
> held in an instance field is in the same performance band. After C2, only a
> JIT-constant callee (direct {{invokevirtual}} / {{invokestatic}} /
> {{invokeinterface}} in generated bytecode, or {{invokeExact}} of a {{static
> final}} / classData handle, or a linked {{invokedynamic}} CallSite) runs like
> a Java direct call.
> Hot monomorphic indy and {{@CompileStatic}} already have that shape.
> {{CachedMethod.invoke}} does not.
> h2. Approach
> After {{groovy.cachedmethod.invoker.threshold}} hits (default 100, below
> {{groovy.indy.optimize.threshold}} of 1000 so cold indy is still on
> {{doMethodInvoke}} when the trampoline appears), install a generated
> {{DirectInvoker}} behind {{CachedMethod.invoke}} only.
> Internal types live in {{org.apache.groovy.internal.runtime.invoke}}
> (japicmp-excluded). Definition reuses {{HiddenClassDefiner}} (GROOVY-12223)
> and {{ClassLoaderForClassArtifacts}}.
> Define order:
> # InvokerFactory nestmate + direct invoke when the member is publicly
> invocable from that class ({{String.startsWith}}).
> # Declaring-class nestmate + direct invoke when {{privateLookupIn}} is
> possible. Private class methods use {{invokevirtual}}; private interface
> methods use {{invokeinterface}} (hidden nestmates do not subclass the host,
> so {{invokespecial}} fails verification).
> # InvokerFactory nestmate + classData {{MethodHandle}} + {{invokeExact}} when
> types are still resolvable from the runtime loader.
> # {{ClassLoaderForClassArtifacts}} when the host loader can resolve
> {{DirectInvoker}} — never for bootstrap types.
> Failures sticky-return {{null}}; {{CachedMethod.invoke}} keeps
> {{Method.invoke}}. Generation is skipped for caller-sensitive and abstract
> methods, Android, native image, and when hidden classes are disabled.
> This is the MOP "Groovy as caller" path ({{makeAccessible}}). Indy continues
> to {{unreflect}} with the call-site {{Lookup}} and must not be fed the
> trampoline.
> h2. Configuration
> {noformat}
> -Dgroovy.cachedmethod.invoker.threshold=100
> -Dgroovy.cachedmethod.invoker.disable=true
> {noformat}
> The existing {{-Dgroovy.hidden.classes.disable=true}} also turns generation
> off.
> h2. Compatibility
> * No change to the {{MetaMethod.invoke}} / {{CachedMethod.invoke}} signatures.
> * Selection (categories, EMC, interceptable, per-instance MetaClass) is
> unchanged; the trampoline is bound to the Java {{Method}}, not to a
> {{MetaMethod}} wrapper.
> * Wrong-argument type on the generated path is {{ClassCastException}}
> (rethrown), matching DGM / {{CallSiteGenerator}}. The reflective path still
> wraps {{IllegalArgumentException}} in {{InvokerInvocationException}}.
> * Opt-out: {{-Dgroovy.cachedmethod.invoker.disable=true}}.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
