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

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

paulk-asert opened a new pull request, #2762:
URL: https://github.com/apache/groovy/pull/2762

   …packed closures work in native images
   
   Two layered changes to the GROOVY-12151 packed-closure machinery (GEP-27):
   
   1. ClosureWriter now passes the three dispatch tables as constant bootstrap 
arguments (CONSTANT_MethodHandle), resolved by the VM's constant pool rather 
than a runtime Lookup.findStatic 

> 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.
> 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 already 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. 
> So this ticket only affects users who have already crossed that larger hurdle.
> 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. Proposed fix (prototyped)
> Two layered changes. JVM semantics are unchanged.
> *1. Constant bootstrap arguments (ClosureWriter).* Emit the three dispatch 
> tables as {{CONSTANT_MethodHandle}} bootstrap arguments, as javac does for 
> LMF's {{implMethod}}:
> {noformat}
> invokedynamic packedDispatchers()Object
>     bsm  = IndyInterface.packedDispatchers(Lookup, String, MethodType,
>                                            MethodHandle, MethodHandle, 
> MethodHandle)
>     args = [ MH(invokestatic host.$packedDispatch$), MH(...1$), MH(...2$) ]
> {noformat}
> Constant-pool method handles are resolved by the VM (and pre-resolved at 
> image build time), so linking needs no {{findStatic}} and no metadata. 
> Verified: the tracing agent records *zero* {{packedDispatch}} entries for the 
> new bytecode. The existing 3-arg bootstrap is kept for class files emitted by 
> earlier 6.0 snapshots.
> *2. Method-handle bundle fallback (GeneratedDispatcher).* On a regular JVM 
> the {{LambdaMetafactory}} hidden-class adapters are kept — their interface 
> call inlines under the JIT, which is the documented rationale. Where classes 
> cannot be defined at run time, the tables are wrapped in 
> method-handle-invoking adapters instead: ordinary Java lambdas of 
> {{GeneratedDispatcher}} itself, whose bytecode-level LMF sites are 
> AOT-compiled into the image, so nothing is defined at run time.
> Detection is 
> {{"runtime".equals(System.getProperty("org.graalvm.nativeimage.imagecode"))}},
>  evaluated *per link* rather than cached in a static — under native image the 
> class may be build-time initialized, where the property reports 
> {{buildtime}}, and caching would bake the wrong answer into the image heap. A 
> catch-based fallback around the metafactory calls covers AOT runtimes the 
> property probe misses.
> One subtlety: dispatch targets may throw checked exceptions the dispatcher 
> interfaces do not declare, and the hidden-class path propagates them 
> transparently. The wrapper path rethrows via {{UncheckedThrow}} to match; a 
> dedicated test covers this.
> h2. Opt-in / opt-out / permanent?
> Permanent and automatic — no user-facing flag:
> * JVM behaviour is byte-identical; the fallback engages only where the 
> hidden-class path *cannot work*. A flag would choose between "works" and 
> "crash".
> * The gate already exists one level up: packing is itself opt-in 
> ({{groovy.target.closure.pack}} / {{@PackedClosures}}) and experimental under 
> GEP-27.
> * {{-Dgroovy.packed.dispatch.handles=true}} exists as a *diagnostic* knob 
> only, forcing the wrapper path on a JVM so CI can assert parity without a 
> native build.
> h2. Correctness verification
> ||check||result||
> |{{PackedDispatcherHandleBundleTest}} (new): parity across all three dispatch 
> shapes and the GDK; undeclared checked-exception propagation|2/2 pass|
> |existing packed/closure suites ({{PackedClosuresTransformTest}}, 
> {{ClosurePackCapabilityTest}}, {{PackedClosureBoundariesTest}}, 
> {{PackedClosureMetaClassTest}}, {{PackedClosureDebugMetadataTest}}, 
> {{ClosureAndInnerClassNodeStructureTest}}, {{ClosureCallMopGuardTest}})|57 
> tests green|
> |repro, unpatched master, native|{{Classes cannot be defined at runtime}}|
> |repro, patched, native|runs correctly, exit 0, no exceptions|
> |agent metadata for the dispatcher|0 entries (was: per-class method 
> registrations)|
> h2. Performance
> Measured on a 120-closure workload (121 classes unpacked, 1 packed) and a 
> dispatch-heavy loop, GraalVM CE 25.2.4 / native-image 25.0.4. *The honest 
> summary: this fix is cheap, but packing in a native image is a trade rather 
> than a win.*
> *Cost of the fix itself.* Isolated by building the same packed sources 
> against unpatched master (which builds, then fails at run time) and against 
> the patch:
> ||build||image size||delta||
> |unpacked|29,138,784 bytes| |
> |packed, unpatched master (does not run)|30,528,208 bytes|+1.39 MB|
> |packed, patched (runs)|30,643,936 bytes|+1.51 MB|
> So the fix adds *~113 KB (+0.4%)*. The larger +1.39 MB is pre-existing 
> packing overhead, not introduced here: {{jdk.internal.classfile.impl}} (561 
> KiB) is pulled in by the {{LambdaMetafactory}} reference that master already 
> had.
> *Image size.* Packing makes the image ~1.4 MB *bigger*, not smaller. The 
> 121-to-1 class collapse does not translate into image savings — native image 
> already handles many small classes efficiently, and the {{PackedClosure}} 
> runtime is a fixed cost. (An earlier draft of this ticket asserted the 
> opposite; the measurements above correct it.)
> *Steady-state dispatch.*
> ||workload||native||JVM||
> |unpacked|20–21 ms|18–21 ms|
> |packed|*35–41 ms (~2x slower)*|19–20 ms|
> |packed, MH path forced on JVM|—|20.2–20.4 ms (~7% over hidden-class)|
> The JVM is unaffected: the wrapper costs ~7% there and is not used anyway. In 
> an image there is no JIT to fold the {{invokeExact}}, so the indirection 
> shows up in full on this deliberately dispatch-heavy loop; ordinary code will 
> see less.
> *Startup*, where packing does win — fewer classes to initialize:
> ||workload||unpacked||packed||
> |120 closures|21.9 ms|*11.6 ms*|
> |6 closures|12.8 ms|12.5 ms|
> h2. Follow-ups (not this ticket)
> * *Drop the LMF reference on the native path* so 
> {{jdk.internal.classfile.impl}} (561 KiB) falls out of the image, removing 
> most of packing's size overhead.
> * *A class-free dispatch that is not method-handle-based* — for example 
> {{MethodHandles.tableSwitch}}, or a generated switch — to close the ~2x 
> steady-state gap. That would turn native packing from a trade into a win.
> * *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