paulk-asert commented on PR #2645:
URL: https://github.com/apache/groovy/pull/2645#issuecomment-5086688193
## Follow-up (a): done — preemption-check cost measured, gate landed
Resolving the compile-time question I flagged above. Short version: the
concern was real, the gate fixes it, and it's now measured rather than asserted.
### Benchmark
Added `ClassTagResolutionBench` (JMH, in `:performance`) — a generated
`@CompileStatic` corpus compiled to `INSTRUCTION_SELECTION`, fresh
`CompilationUnit` per invocation. Three call mixes and a controlled `active` vs
`vetoAll` comparison, where `vetoAll` sets `classTagPreemptionDisabled=['*']`
so the preemption path is never entered — the pre-feature floor for a corpus
whose calls all resolve. `active − vetoAll` is therefore the cost of active
preemption matching. The corpus and the numbers below live in the benchmark's
Javadoc so they travel with the code.
### The regression, confirmed
Without the gate, preemption matching runs on every resolved call once a
token-less overload has matched:
```
mix active vetoAll(floor) delta
dgmHeavy 121.8 109.7 +11.1% <- ordinary
each/collect/findAll code
instanceHeavy 52.8 51.0 + 3.6%
withDefault 263.1 228.7 +15.0% <- preemption actually
firing every call
```
*(avgt ms/op, 60 classes × 80 calls, 2 forks × 5+5 iterations.)*
The `dgmHeavy` +11% is the pathology: `each`/`collect`/`findAll` on typed
receivers each pay a full DGM candidate scan + intent check, only to find
nothing preemptive (no DGM method except `withDefault` declares intent). Pure
overhead — well past the ~1–2% I'd have shipped as-is on.
### The gate
`ExtensionMethodCache` now exposes `getPreemptiveNames(loader)` — a
per-classloader `Set<String>` of extension-method names that declare
`@ClassTag(preempt=true)`, derived lazily from the already-cached method map
and sharing its lifecycle (so it can never go stale independently). Out of the
box it holds exactly `{"withDefault"}`. `matchPreemption` consults it before
the extension candidate scan: if the call's name isn't in the set, one
`HashSet.contains` replaces the whole scan+filter. Conservative by construction
— a false positive falls through to the full match and fails softly; a false
negative can't occur because any intent-declaring overload's name is in the set
by derivation. The instance-incumbent path is untouched (it must still see
subclass-preempts-superclass), and additive matching is untouched (it only runs
on otherwise-unresolved calls).
### Post-gate
```
mix active(gated) vetoAll(floor) pre-gate
dgmHeavy 108.1 110.3 121.8 <- regression
eliminated (at the floor)
instanceHeavy 54.6 51.4 52.8
withDefault 269.7 227.0 263.1 <- unchanged; still
does the real work
```
`dgmHeavy` gated `active` is back at the `vetoAll` floor, within noise — the
+11% is gone. `withDefault` is deliberately unchanged: it's in the
preemptive-names set, so it still does the full token synthesis + rewrite the
feature exists to do (this number is a worst case — *every* call preempts,
which no real code does).
### Diff and verification
Three files: the benchmark (new), `AbstractExtensionMethodCache` (+21, the
name-set cache), `ClassTagSupport` (+7, the guard). `ClassTagStaticTest` 32/32,
`CompilerConfigurationTest` 15/15, `Groovy11807` 5/5, and the full
`groovy.transform.stc.*` (2012 tests) + `classgen.asm.sc.*` (2295 tests) sweeps
green — the shared-cache change is safe.
### One documented residual
`instanceHeavy` still shows ~+6% (gated 54.6 vs floor 51.4) on a corpus
where *every* call is to a same-class overloaded method. The gate deliberately
doesn't cover the instance path — a per-`ClassNode` equivalent runs into
invalidation-during-compilation complexity, and real instance calls are
overwhelmingly non-overloaded (the scan returns one method, trivial). I've left
it as a documented residual rather than adding speculative `ClassNode`-metadata
caching; happy to revisit if anyone has a real-world `instanceHeavy` profile
that justifies it.
--
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]
