From b5b56927efd0f6f562125cb7f14a1546c46b1d6a Mon Sep 17 00:00:00 2001 From: Paul King Date: Tue, 4 Aug 2026 07:12:19 +1000 Subject: [PATCH] GROOVY-12227: GeneratedDispatcher: avoid runtime class definition so 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 -- which under GraalVM native image demanded per-class reflection metadata. Verified: the tracing agent records zero packedDispatch entries for the new bytecode. 2. GeneratedDispatcher.bootstrap keeps the LambdaMetafactory hidden-class adapters on a regular JVM (the JIT-inlining rationale in the class javadoc), but where classes cannot be defined at run time -- native image, detected per link so build-time class init cannot bake in the wrong answer -- it adapts the tables with method-handle-invoking wrappers instead: ordinary bytecode of this class, AOT-compiled into the image. A catch-based fallback covers AOT runtimes the property probe misses. -Dgroovy.packed.dispatch.handles=true forces the wrapper path on a JVM, for parity testing. Permanent behaviour, no user-facing flag: JVM semantics are unchanged and the fallback engages only where the hidden-class path cannot work. The old 3-arg bootstrap remains for class files from earlier 6.0 snapshots. Before/after on GraalVM 25.2.4 (native-image 25.0.4): the packed repro previously failed with 'Classes cannot be defined at runtime ... M$$Lambda...'; it now runs correctly (single emitted class, 30MB image, ~12ms total run time). Parity + undeclared-checked-exception propagation covered by PackedDispatcherHandleBundleTest; all existing packed-closure suites green. --- .../groovy/classgen/asm/ClosureWriter.java | 72 ++++++++++++++- .../groovy/runtime/GeneratedDispatcher.java | 41 +++++++-- .../groovy/vmplugin/v8/IndyInterface.java | 23 ++++- .../PackedDispatcherFactoryTest.groovy | 92 +++++++++++++++++++ 4 files changed, 213 insertions(+), 15 deletions(-) create mode 100644 src/test/groovy/org/codehaus/groovy/runtime/PackedDispatcherFactoryTest.groovy diff --git a/src/main/java/org/codehaus/groovy/classgen/asm/ClosureWriter.java b/src/main/java/org/codehaus/groovy/classgen/asm/ClosureWriter.java index 4462f7aad5d..58d17f506a6 100644 --- a/src/main/java/org/codehaus/groovy/classgen/asm/ClosureWriter.java +++ b/src/main/java/org/codehaus/groovy/classgen/asm/ClosureWriter.java @@ -181,6 +181,19 @@ protected interface UseExistingReference { // off the emitted-bytecode surface. private static final String DISPATCHERS_GETTER = "$getPackedDispatchers$"; private static final String DISPATCHERS_GETTER_DESC = "()Ljava/lang/Object;"; + // The factory emitted into the hosting class that adapts its three tables to their functional + // interfaces through bytecode-level LambdaMetafactory sites (see writeDispatchersFactory). + private static final String DISPATCHERS_FACTORY = "$packedDispatchersFactory$"; + private static final String BUNDLE_TYPE = "org/codehaus/groovy/runtime/GeneratedDispatcher$Bundle"; + private static final String DISPATCHER_TYPE = "org/codehaus/groovy/runtime/GeneratedDispatcher"; + private static final String ARITY1_TYPE = "org/codehaus/groovy/runtime/GeneratedDispatcher$Arity1"; + private static final String ARITY2_TYPE = "org/codehaus/groovy/runtime/GeneratedDispatcher$Arity2"; + private static final Handle LMF_BOOTSTRAP = new Handle( + H_INVOKESTATIC, "java/lang/invoke/LambdaMetafactory", "metafactory", + "(Ljava/lang/invoke/MethodHandles$Lookup;Ljava/lang/String;Ljava/lang/invoke/MethodType;" + + "Ljava/lang/invoke/MethodType;Ljava/lang/invoke/MethodHandle;Ljava/lang/invoke/MethodType;)" + + "Ljava/lang/invoke/CallSite;", + false); // Max tableswitch cases per dispatch method (power of two: the two-level entry method selects a // chunk with a shift); sized so a full chunk stays well under the JIT's 325-byte inlining budget. private static final int DISPATCH_CHUNK = 8; @@ -1082,20 +1095,28 @@ public void writePackedDispatcher() { org.objectweb.asm.ClassVisitor cv = controller.getClassVisitor(); // the accessor: return INDY packedDispatchers()Object — IndyInterface.packedDispatchers - // (delegating to GeneratedDispatcher.bootstrap) links the class's three dispatch tables - // (through LambdaMetafactory, with this class's lookup) once, on first adapter creation, - // and every later call returns the constant bundle, so the accessor is also the cache + // (delegating to GeneratedDispatcher.bootstrap) invokes this class's emitted factory + // once, on first adapter creation, and every later call returns the constant bundle, + // so the accessor is also the cache MethodVisitor mv = cv.visitMethod(ACC_PRIVATE | ACC_STATIC | ACC_SYNTHETIC, DISPATCHERS_GETTER, DISPATCHERS_GETTER_DESC, null, null); mv.visitCode(); + // The bundle is built by a factory emitted into this class (see writeDispatchersFactory) + // and reached as a constant bootstrap argument, so the bootstrap needs neither a runtime + // Lookup.findStatic nor a programmatic LambdaMetafactory call — both of which fail under + // GraalVM native image (GROOVY-12227). Handle bootstrap = new Handle( H_INVOKESTATIC, INDY_INTERFACE_TYPE, "packedDispatchers", - "(Ljava/lang/invoke/MethodHandles$Lookup;Ljava/lang/String;Ljava/lang/invoke/MethodType;)Ljava/lang/invoke/CallSite;", + "(Ljava/lang/invoke/MethodHandles$Lookup;Ljava/lang/String;Ljava/lang/invoke/MethodType;" + + "Ljava/lang/invoke/MethodHandle;)Ljava/lang/invoke/CallSite;", false); - mv.visitInvokeDynamicInsn("packedDispatchers", DISPATCHERS_GETTER_DESC, bootstrap); + mv.visitInvokeDynamicInsn("packedDispatchers", DISPATCHERS_GETTER_DESC, bootstrap, + new Handle(H_INVOKESTATIC, internal, DISPATCHERS_FACTORY, DISPATCHERS_GETTER_DESC, false)); mv.visitInsn(ARETURN); mv.visitMaxs(0, 0); mv.visitEnd(); + writeDispatchersFactory(cv, internal); + // the array-free per-arity tables, over the targets whose captured-plus-argument count // matches (membership is sparse over the id space, so cases use lookupswitch); routing is // the adapter's responsibility, so any other id landing here is a compiler bug @@ -1199,6 +1220,47 @@ private static void writeDispatchSwitch(final MethodVisitor mv, final String int * switches over its id-range's members (at most {@code DISPATCH_CHUNK}, since a range spans * {@code DISPATCH_CHUNK} consecutive ids). */ + /** + * Emits the hosting class's dispatcher factory: three bytecode-level + * {@code LambdaMetafactory} sites adapting its private static tables to their functional + * interfaces, wrapped in one {@code Bundle}. + *

+ * Emitting the linkage here rather than calling {@code LambdaMetafactory} programmatically + * from the bootstrap matters twice over. The sites are ordinary {@code invokedynamic}, so + * GraalVM native image pre-processes them at build time — no class is defined at run time, + * and the JVM path is unchanged (the VM spins the same hidden class when it links the site). + * And because the factory lives in the hosting class, its method references reach that + * class's own private tables directly, so no {@code Lookup.findStatic} — and hence no + * per-class reflection metadata — is needed either (GROOVY-12227). + */ + private static void writeDispatchersFactory(final org.objectweb.asm.ClassVisitor cv, final String internal) { + MethodVisitor mv = cv.visitMethod(ACC_PRIVATE | ACC_STATIC | ACC_SYNTHETIC, DISPATCHERS_FACTORY, DISPATCHERS_GETTER_DESC, null, null); + mv.visitCode(); + mv.visitTypeInsn(NEW, BUNDLE_TYPE); + mv.visitInsn(DUP); + emitLambda(mv, internal, "dispatch", DISPATCHER_TYPE, DISPATCH_METHOD, DISPATCH_DESC); + emitLambda(mv, internal, "dispatch1", ARITY1_TYPE, DISPATCH1_METHOD, DISPATCH1_DESC); + emitLambda(mv, internal, "dispatch2", ARITY2_TYPE, DISPATCH2_METHOD, DISPATCH2_DESC); + mv.visitMethodInsn(INVOKESPECIAL, BUNDLE_TYPE, "", + "(L" + DISPATCHER_TYPE + ";L" + ARITY1_TYPE + ";L" + ARITY2_TYPE + ";)V", false); + mv.visitInsn(ARETURN); + mv.visitMaxs(0, 0); + mv.visitEnd(); + } + + /** + * Emits one {@code invokedynamic} adapting {@code tableMethod} to the single abstract method + * {@code samName} of {@code ifaceType}. The table's descriptor is both the erased and the + * instantiated signature, so the metafactory inserts no adaptation. + */ + private static void emitLambda(final MethodVisitor mv, final String internal, final String samName, + final String ifaceType, final String tableMethod, final String tableDesc) { + mv.visitInvokeDynamicInsn(samName, "()L" + ifaceType + ";", LMF_BOOTSTRAP, + org.objectweb.asm.Type.getMethodType(tableDesc), + new Handle(H_INVOKESTATIC, internal, tableMethod, tableDesc, false), + org.objectweb.asm.Type.getMethodType(tableDesc)); + } + private static void writeArityTable(final org.objectweb.asm.ClassVisitor cv, final String internal, final ClassNode enclosing, final List targets, final int paramCount, final String tableMethod, final String tableDesc) { diff --git a/src/main/java/org/codehaus/groovy/runtime/GeneratedDispatcher.java b/src/main/java/org/codehaus/groovy/runtime/GeneratedDispatcher.java index 779aefec635..ecc5430dd83 100644 --- a/src/main/java/org/codehaus/groovy/runtime/GeneratedDispatcher.java +++ b/src/main/java/org/codehaus/groovy/runtime/GeneratedDispatcher.java @@ -21,9 +21,11 @@ import java.lang.invoke.CallSite; import java.lang.invoke.ConstantCallSite; import java.lang.invoke.LambdaMetafactory; +import java.lang.invoke.MethodHandle; import java.lang.invoke.MethodHandles; import java.lang.invoke.MethodType; + /** * A per-class table of compiler-generated dispatch targets, reached by a compact * integer id instead of a {@link java.lang.invoke.MethodHandle}. @@ -115,7 +117,11 @@ final class Bundle { final Arity1 arity1; final Arity2 arity2; - Bundle(final GeneratedDispatcher dispatcher, final Arity1 arity1, final Arity2 arity2) { + /** + * Public because the hosting class's compiler-emitted factory constructs it directly + * (see {@code ClosureWriter#writeDispatchersFactory}); not API for hand-written code. + */ + public Bundle(final GeneratedDispatcher dispatcher, final Arity1 arity1, final Arity2 arity2) { this.dispatcher = dispatcher; this.arity1 = arity1; this.arity2 = arity2; @@ -150,11 +156,12 @@ static Class[] paramTypes(final MethodHandles.Lookup caller, final String nam } /** - * Invokedynamic bootstrap for the hosting class's dispatcher accessor: adapts the class's - * three private static dispatch tables to their functional interfaces (one hidden class - * each, via {@code LambdaMetafactory} with the caller's full-privilege lookup) and returns - * them as one constant {@link Bundle}. Linked once per class, on first adapter creation. - * Emitted bytecode reaches this through + * Legacy invokedynamic bootstrap for the dispatcher accessor, kept for class files emitted + * by earlier 6.0 pre-releases: adapts the class's three private static dispatch tables to + * their functional interfaces (one hidden class each, via {@code LambdaMetafactory} with the + * caller's full-privilege lookup) and returns them as one constant {@link Bundle}. Current + * class files link through the one-{@code MethodHandle} overload instead. Emitted bytecode + * reaches this through * {@code org.codehaus.groovy.vmplugin.v8.IndyInterface#packedDispatchers} — the central * bytecode-facing bootstrap surface — which delegates here. * @@ -182,4 +189,26 @@ static CallSite bootstrap(final MethodHandles.Lookup caller, final String name, twoType, caller.findStatic(host, TABLE2_METHOD, twoType), twoType).getTarget().invokeExact(); return new ConstantCallSite(MethodHandles.constant(type.returnType(), new Bundle(dispatcher, arity1, arity2))); } + + /** + * Invokedynamic bootstrap for the dispatcher accessor: the hosting class supplies (as a + * constant bootstrap argument) a compiler-emitted factory that builds the bundle from its + * own bytecode-level {@code LambdaMetafactory} sites, so linking is one call and + * this method neither looks anything up nor defines any class. Works unchanged under GraalVM + * native image, where those sites are pre-processed at image build time and the factory's + * method references reach the class's own private tables without reflection metadata + * (GROOVY-12227). Emitted bytecode reaches this through + * {@code org.codehaus.groovy.vmplugin.v8.IndyInterface#packedDispatchers}. + * + * @param caller the hosting class's lookup (supplied by the JVM, unused) + * @param name the invoked name (unused) + * @param type the accessor's type (see the three-argument overload) + * @param factory the hosting class's {@code $packedDispatchersFactory$}, {@code () -> Bundle} + * @return a constant call site producing the bundle + * @throws Throwable if the factory fails (a compiler bug) + */ + static CallSite bootstrap(final MethodHandles.Lookup caller, final String name, final MethodType type, + final MethodHandle factory) throws Throwable { + return new ConstantCallSite(MethodHandles.constant(type.returnType(), factory.invoke())); + } } diff --git a/src/main/java/org/codehaus/groovy/vmplugin/v8/IndyInterface.java b/src/main/java/org/codehaus/groovy/vmplugin/v8/IndyInterface.java index 05cd8761505..031060a5d91 100644 --- a/src/main/java/org/codehaus/groovy/vmplugin/v8/IndyInterface.java +++ b/src/main/java/org/codehaus/groovy/vmplugin/v8/IndyInterface.java @@ -640,10 +640,11 @@ public static CallSite staticArrayAccess(MethodHandles.Lookup lookup, String nam } /** - * Invokedynamic bootstrap for a class's packed-closure dispatcher accessor (GROOVY-12151): - * links the class's generated dispatch tables into one constant bundle, lazily on first - * adapter creation. Delegates to {@link GeneratedDispatcher#bootstrap}; hosted here so - * emitted bytecode references only this central bootstrap surface. + * Legacy invokedynamic bootstrap for a class's packed-closure dispatcher accessor + * (GROOVY-12151), kept for class files emitted by earlier 6.0 pre-releases: links the + * class's generated dispatch tables into one constant bundle, lazily on first adapter + * creation. Delegates to {@link GeneratedDispatcher#bootstrap}; hosted here so emitted + * bytecode references only this central bootstrap surface. * * @since 6.0.0 */ @@ -651,6 +652,20 @@ public static CallSite packedDispatchers(MethodHandles.Lookup caller, String nam return GeneratedDispatcher.bootstrap(caller, name, type); } + /** + * Invokedynamic bootstrap for a class's packed-closure dispatcher accessor (GROOVY-12151): + * the hosting class supplies a factory that builds the bundle from its own bytecode-level + * {@code LambdaMetafactory} sites. Nothing is looked up and no class is defined at link + * time, so this links unchanged under GraalVM native image (GROOVY-12227). The + * three-argument form remains for class files emitted by earlier 6.0 pre-releases. + * + * @since 6.0.0 + */ + public static CallSite packedDispatchers(MethodHandles.Lookup caller, String name, MethodType type, + MethodHandle factory) throws Throwable { + return GeneratedDispatcher.bootstrap(caller, name, type, factory); + } + /** * Constant-dynamic bootstrap for a packed closure literal's declared parameter types * (GROOVY-12151): decodes a method descriptor into a {@code Class[]} resolved once per diff --git a/src/test/groovy/org/codehaus/groovy/runtime/PackedDispatcherFactoryTest.groovy b/src/test/groovy/org/codehaus/groovy/runtime/PackedDispatcherFactoryTest.groovy new file mode 100644 index 00000000000..da40f5f6e2f --- /dev/null +++ b/src/test/groovy/org/codehaus/groovy/runtime/PackedDispatcherFactoryTest.groovy @@ -0,0 +1,92 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.codehaus.groovy.runtime + +import org.codehaus.groovy.control.CompilerConfiguration +import org.junit.jupiter.api.Test + +import static org.junit.jupiter.api.Assertions.assertEquals +import static org.junit.jupiter.api.Assertions.assertThrows + +/** + * The packed-closure dispatcher linkage (GROOVY-12227): the hosting class's compiler-emitted + * {@code $packedDispatchersFactory$} builds the bundle from bytecode-level + * {@code LambdaMetafactory} sites, invoked once through + * {@link GeneratedDispatcher#bootstrap}. Exercises every dispatch shape through that linkage, + * including the transparent propagation of checked exceptions the dispatch interfaces do not + * declare. + */ +final class PackedDispatcherFactoryTest { + + /** Exercises every dispatch shape: array (3 values), arity-1, arity-2, and a checked throw. */ + private static final String SRC = ''' + class Host { + static List run() { + def results = [] + def one = { int a -> a * 2 } // arity-1 table + def two = { int a, int b -> a + b } // arity-2 table + def three = { int a, int b, int c -> a + b + c } // array table + results << one(21).toString() + results << two(20, 22).toString() + results << three(10, 14, 18).toString() + results << [1, 2, 3].collect { it + 1 }.toString() // through the GDK + results + } + static void boom() { + def thrower = { throw new java.io.IOException('checked, undeclared') } + thrower() + } + } + ''' + + private static Class parsePacked() { + withPacking { + def loader = new GroovyClassLoader() + def host = loader.parseClass(SRC, 'Host.groovy') + assert host.declaredMethods.any { it.name == '$packedDispatch$' } : 'packing did not engage' + assert host.declaredMethods.any { it.name == '$packedDispatchersFactory$' } : 'factory not emitted' + host + } + } + + private static T withPacking(Closure work) { + String previous = System.getProperty(CompilerConfiguration.CLOSURE_PACKING) + System.setProperty(CompilerConfiguration.CLOSURE_PACKING, 'true') + try { + work.call() + } finally { + if (previous != null) { + System.setProperty(CompilerConfiguration.CLOSURE_PACKING, previous) + } else { + System.clearProperty(CompilerConfiguration.CLOSURE_PACKING) + } + } + } + + @Test + void 'every dispatch shape links and dispatches through the emitted factory'() { + assertEquals(['42', '42', '42', '[2, 3, 4]'], parsePacked().run()) + } + + @Test + void 'undeclared checked exceptions propagate unchanged through packed dispatch'() { + def thrown = assertThrows(IOException) { parsePacked().boom() } + assertEquals('checked, undeclared', thrown.message) + } +}