[
https://issues.apache.org/jira/browse/GROOVY-12227?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18101596#comment-18101596
]
ASF GitHub Bot commented on GROOVY-12227:
-----------------------------------------
testlens-app[bot] commented on PR #2765:
URL: https://github.com/apache/groovy/pull/2765#issuecomment-5175298906
## ✅ All tests passed ✅
🏷️ Commit: b5b56927efd0f6f562125cb7f14a1546c46b1d6a
▶️ Tests: 108745 executed
⚪️ Checks: 31/31 completed
---
_Learn more about TestLens at [testlens.app](https://testlens.app)._
> GeneratedDispatcher: avoid runtime class definition so packed closures work
> in native images
> ---------------------------------------------------------------------------------------------
>
> Key: GROOVY-12227
> URL: https://issues.apache.org/jira/browse/GROOVY-12227
> Project: Groovy
> Issue Type: Improvement
> Reporter: Paul King
> Assignee: Paul King
> Priority: Major
>
> Packed closures (GROOVY-12151, GEP-27) cannot be used in a GraalVM native
> image. A class compiled with {{groovy.target.closure.pack=true}} links its
> dispatch tables on first closure creation, and that linkage fails inside an
> image — in two layers.
> Without agent-recorded metadata, the bootstrap's method lookup fails:
> {noformat}
> java.lang.BootstrapMethodError: java.lang.NoSuchMethodException:
> no such method: B.$packedDispatch$(int,Object[])Object/invokeStatic
> {noformat}
> With agent-recorded metadata the lookup succeeds, and the real wall appears —
> the bootstrap spins hidden classes at run time, which an image forbids:
> {noformat}
> Exception in thread "main" java.lang.BootstrapMethodError:
> java.lang.InternalError:
> com.oracle.svm.core.jdk.UnsupportedFeatureError: Classes cannot be defined at
> runtime
> by default when using ahead-of-time Native Image compilation. Tried to define
> class:
> B$$LambdadLbm0tKUrZ8kVMiSpulnyK
> {noformat}
> The failure mode is the problem as much as the failure: packing compiles
> cleanly, the image builds cleanly, and the process then dies on the first
> closure created. There is no diagnostic pointing at the flag that caused it,
> and the guidance cannot be enforced at compile time — the class files are
> identical whether or not a native image is built later. The realistic path
> into this is enabling packing globally for its JVM benefit and adding a
> native build afterwards. It also matters for GEP-27's plan to flip packing
> default-on in Groovy 7: at that point this stops being an opt-in corner and
> becomes every Groovy native image with a closure in it.
> h2. Reproducing
> {code:java}
> import groovy.transform.CompileStatic
> @CompileStatic
> class B {
> static int work(int n) {
> def xs = (1..n).toList()
> int total = 0
> total += xs.collect { int v -> v * 2 }.size()
> total += xs.findAll { int v -> v % 2 == 0 }.size()
> total += (xs.inject(0) { int a, int b -> a + b } as int)
> total
> }
> static void main(String[] args) { println "result=${work(20)}" }
> }
> {code}
> {noformat}
> JAVA_OPTS="-Dgroovy.target.indy=false -Dgroovy.target.closure.pack=true" \
> groovyc -cp groovy-callsite.jar B.groovy # emits a single class:
> B.class
> java -agentlib:native-image-agent=config-output-dir=cfg -cp
> "groovy.jar:groovy-callsite.jar:." B
> native-image --no-fallback -H:ConfigurationFileDirectories=cfg \
>
> -H:IncludeResources='META-INF/dgminfo|META-INF/groovy/.*|META-INF/services/.*'
> \
> -cp "groovy.jar:groovy-callsite.jar:." B
> ./b # fails as above
> {noformat}
> Reproduced with GraalVM CE 25.2.4 (native-image 25.0.4 — the first release
> whose Groovy substitution guard is fixed for Groovy 5+, see
> oracle/graal#13096; earlier releases fail before reaching this).
> Note that indy must currently be off, with {{groovy-callsite}} on the
> *compile* classpath: Groovy's own {{IndyInterface}} dispatch uses mutable
> call sites, which native image does not support, and it fails upstream of
> anything here. Whether that larger hurdle can also be removed is under
> separate investigation; either way, closure dispatch has to link natively for
> any of it to matter.
> h2. Root cause
> {{GeneratedDispatcher.bootstrap}} does two things an image cannot support:
> # {{caller.findStatic(host, "$packedDispatch$", ...)}} — a *runtime*
> method-handle lookup, which under native image needs per-user-class
> reflection metadata. Groovy's own jar cannot ship that, so every user would
> need an agent run.
> # Three *programmatic*
> {{LambdaMetafactory.metafactory(...).getTarget().invokeExact()}} calls, each
> defining a hidden class at run time. Bytecode-level LMF invokedynamic is
> pre-processed at image build time and works fine; programmatic metafactory
> calls inside a bootstrap are runtime class definition, which is forbidden.
> The surrounding design is already image-friendly: the result is wrapped in a
> {{ConstantCallSite}} and never re-linked.
> h2. Fix (prototyped)
> Move the linkage from the bootstrap into the class file. {{ClosureWriter}}
> emits one extra synthetic method into the hosting class:
> {noformat}
> private static synthetic Object $packedDispatchersFactory$() {
> return new Bundle(
> indy LMF[Host::$packedDispatch$], // GeneratedDispatcher
> indy LMF[Host::$packedDispatch1$], // Arity1
> indy LMF[Host::$packedDispatch2$]); // Arity2
> }
> {noformat}
> and the accessor's {{invokedynamic}} passes that factory to the bootstrap as
> a single {{CONSTANT_MethodHandle}} bootstrap argument. The bootstrap becomes
> one line — invoke the factory, wrap the bundle in a {{ConstantCallSite}}.
> Why this solves both layers at once:
> * The three {{LambdaMetafactory}} sites are ordinary *bytecode-level*
> invokedynamic, so native image pre-processes them at build time — no class is
> defined at run time. On a regular JVM the VM spins the same hidden classes it
> always did, at site-link time instead of from the bootstrap; the JIT-inlining
> rationale for the hidden-class adapters (see the {{GeneratedDispatcher}}
> javadoc) is fully preserved.
> * Because the factory lives in the hosting class, its method references reach
> that class's own private dispatch tables directly — no {{Lookup.findStatic}},
> and hence no per-class reflection metadata. Verified: the tracing agent
> records *zero* {{packedDispatch}} entries for the new bytecode.
> There is deliberately *one* code path for JVM and native — no platform
> detection, no fallback machinery, no flags. (An earlier iteration of this
> prototype kept the programmatic-LMF path and added a method-handle-wrapper
> fallback under native image; it worked, but cost a ~1.4 MB image-size pull-in
> of {{jdk.internal.classfile.impl}} via the retained metafactory reference and
> ~2x dispatch through uninlined {{invokeExact}}. The factory emission
> eliminates the fallback and both costs, so that machinery has been removed. A
> {{MethodHandles.tableSwitch}}-based dispatch was also evaluated and rejected:
> ~75x slower than even the wrapper path under AOT, where the switch combinator
> runs interpreted.)
> The previous three-argument bootstrap is retained for class files emitted by
> earlier 6.0 pre-releases; verified by compiling a packed workload with
> 6.0.0-beta-1 and running the resulting class files against the patched
> runtime.
> h2. Opt-in / opt-out / permanent?
> Permanent and automatic — no user-facing flag. There is a single linkage path
> whose observable behaviour matches the old one on the JVM (same hidden-class
> adapters, same dispatch), and the gate already exists one level up: packing
> is itself opt-in ({{groovy.target.closure.pack}} / {{@PackedClosures}}) and
> experimental under GEP-27.
> h2. Correctness verification
> ||check||result||
> |{{PackedDispatcherFactoryTest}} (new): every dispatch shape (array, arity-1,
> arity-2, GDK) links and dispatches through the emitted factory; undeclared
> checked exceptions propagate unchanged|2/2 pass|
> |existing packed/closure suites ({{PackedClosuresTransformTest}},
> {{ClosurePackCapabilityTest}}, {{PackedClosureBoundariesTest}},
> {{PackedClosureMetaClassTest}}, {{PackedClosureDebugMetadataTest}},
> {{ClosureAndInnerClassNodeStructureTest}}, {{ClosureCallMopGuardTest}})|57
> tests green|
> |wider closure/lambda sweep ({{--tests *Closure* *Lambda*}})|3,147 tests, 0
> failures|
> |repro, unpatched master, native|{{Classes cannot be defined at runtime}}|
> |repro, patched, native|runs correctly, exit 0|
> |agent metadata for the dispatcher|0 entries (was: per-class method
> registrations)|
> |class files compiled by 6.0.0-beta-1 (legacy bootstrap), patched runtime,
> JVM|link and run correctly|
> h2. Performance
> GraalVM CE 25.2.4 / native-image 25.0.4; three workloads (5 closure classes,
> 120 closure classes, a dispatch-heavy loop); startup medians over 41
> interleaved A/B runs to cancel machine drift.
> *Image size.* Packing costs a fixed ~16 KB (+0.06%), independent of closure
> count:
> ||workload||unpacked||packed||delta||
> |5 closures|29,072,496|29,089,056|+16,560|
> |120 closures|29,138,784|29,155,296|+16,512|
> *Steady-state dispatch* (dispatch-heavy loop, native): unpacked median 21.1
> ms, packed 19.5 ms — parity.
> *Startup* scales with how many closure classes collapse:
> ||closure classes||unpacked||packed||change||
> |5 -> 1|16.4 ms|17.1 ms|wash (p5 13.9 vs 14.1)|
> |120 -> 1|29.3 ms|*15.7 ms*|*+46%*|
> Net: with this fix, packing in a native image is a startup win that grows
> with closure-class count, at negligible size cost and no dispatch cost.
> (Without it, packing is a crash.)
> h2. Follow-up (not this ticket)
> * *Ship Groovy's own reachability metadata* (VMPluginFactory reflection,
> {{dgm$NNN}} classes, {{META-INF/dgminfo}}) in the jar, so the agent step
> disappears for everyone. App-independent and pre-existing; unrelated to
> packing.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)