[ 
https://issues.apache.org/jira/browse/GROOVY-12227?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18101585#comment-18101585
 ] 

ASF GitHub Bot commented on GROOVY-12227:
-----------------------------------------

paulk-asert commented on PR #2762:
URL: https://github.com/apache/groovy/pull/2762#issuecomment-5174903108

   Superseded by #2765




> 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)

Reply via email to