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