From 6fcc0a847ab26b419b1b410bba974cc7b4aba734 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 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two layered changes to the GROOVY-12151 packed-closure machinery (GEP-27): 1. ClosureWriter now emits into each packed hosting class a private static $packedDispatchersFactory$ whose body adapts the class's three private static dispatch tables to their functional interfaces through ordinary bytecode-level LambdaMetafactory invokedynamic sites and returns them as one Bundle. The dispatcher accessor's bootstrap becomes the new four-argument IndyInterface.packedDispatchers, which receives that factory as a CONSTANT_MethodHandle bootstrap argument. 2. GeneratedDispatcher.bootstrap (four-argument form) just invokes the factory into a ConstantCallSite: linking needs neither a runtime Lookup.findStatic nor a programmatic LambdaMetafactory call — an undeclared reflective lookup and a run-time class definition, the two operations ahead-of-time runtimes restrict, so the linkage suits any such environment. Under GraalVM native image, the verified case, the factory's sites are pre-processed at image build time and its method references reach the class's own private tables without reflection metadata. On a regular JVM the linkage is equivalent: the VM spins the same three hidden classes when it links the factory's sites, with the cost moving from bootstrap-time programmatic LMF to first-invocation indy linkage (per packed class: one extra synthetic method, three indy sites, one bootstrap argument; the accessor shape is unchanged). The Bundle constructor is publicized because the compiler-emitted factory in the hosting class's own package constructs it directly. The previous three-argument bootstrap is retained verbatim for class files emitted by earlier 6.0 pre-releases, which keep linking through findStatic plus programmatic LambdaMetafactory. Conversely, class files in the new format need this runtime to link; both directions only concern unreleased 6.0 snapshots. Verified on GraalVM 25.2.4 (native-image 25.0.4): the packed repro that previously failed with 'Classes cannot be defined at runtime ... M$$Lambda...' now runs correctly (single emitted class, 30MB image, ~12ms total run time), and the tracing agent records zero packedDispatch entries for the new bytecode. PackedDispatcherFactoryTest covers every dispatch shape and the propagation of undeclared checked exceptions through both class-file formats -- the legacy format recreated by downgrading a freshly compiled accessor's bootstrap reference from the four-argument to the three-argument form, the only difference between the two formats. The test serializes its system-property mutation against other property-touching tests with @ResourceLock(SYSTEM_PROPERTIES). --- .../groovy/classgen/asm/ClosureWriter.java | 74 +++++++- .../groovy/runtime/GeneratedDispatcher.java | 42 ++++- .../groovy/vmplugin/v8/IndyInterface.java | 24 ++- .../PackedDispatcherFactoryTest.groovy | 168 ++++++++++++++++++ 4 files changed, 293 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..9f5d742d8ed 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,29 @@ 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 — an undeclared reflective + // lookup and a run-time class definition, the two operations ahead-of-time runtimes + // restrict; GraalVM native image is the verified case (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 +1221,48 @@ 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}, + * visible in the class file, so an ahead-of-time compiler can pre-process them at build + * time (GraalVM native image, the verified case, does) — 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..021c4b1abc5 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,27 @@ 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 — fit for any ahead-of-time + * runtime that restricts run-time reflection or class definition. Under GraalVM native + * image, the verified case, 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..99d74334ec5 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,21 @@ 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 in ahead-of-time environments that restrict either — + * GraalVM native image being the verified case (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..58fe2058e2d --- /dev/null +++ b/src/test/groovy/org/codehaus/groovy/runtime/PackedDispatcherFactoryTest.groovy @@ -0,0 +1,168 @@ +/* + * 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.CompilationUnit +import org.codehaus.groovy.control.CompilerConfiguration +import org.codehaus.groovy.control.Phases +import org.junit.jupiter.api.Test +import org.junit.jupiter.api.parallel.ResourceLock +import org.junit.jupiter.api.parallel.Resources +import org.objectweb.asm.ClassReader +import org.objectweb.asm.ClassVisitor +import org.objectweb.asm.ClassWriter +import org.objectweb.asm.Handle +import org.objectweb.asm.MethodVisitor +import org.objectweb.asm.Opcodes + +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, and repeats both through the legacy three-argument bootstrap kept for class files + * emitted by earlier 6.0 pre-releases (recreated by downgrading the accessor's bootstrap + * reference, the only difference between the two class-file formats). + */ +@ResourceLock(Resources.SYSTEM_PROPERTIES) +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) + } + } + } + + private static final String LEGACY_BSM_DESC = + '(Ljava/lang/invoke/MethodHandles$Lookup;Ljava/lang/String;Ljava/lang/invoke/MethodType;)Ljava/lang/invoke/CallSite;' + + /** + * Recreates the class-file format of earlier 6.0 pre-releases: the accessor's + * {@code invokedynamic} referenced the three-argument {@code packedDispatchers} bootstrap + * with no bootstrap arguments (runtime {@code Lookup.findStatic} over the table methods, + * which the current format still emits as the factory's implementation methods). Dropping + * the factory bootstrap argument is the only difference between the two formats. + */ + private static byte[] toLegacyFormat(final byte[] modern) { + boolean rewritten = false + def reader = new ClassReader(modern) + def writer = new ClassWriter(reader, 0) + reader.accept(new ClassVisitor(Opcodes.ASM9, writer) { + @Override + MethodVisitor visitMethod(int access, String name, String descriptor, String signature, String[] exceptions) { + new MethodVisitor(Opcodes.ASM9, super.visitMethod(access, name, descriptor, signature, exceptions)) { + @Override + void visitInvokeDynamicInsn(String indyName, String indyDescriptor, Handle bootstrap, Object... bootstrapArguments) { + if (bootstrap.name == 'packedDispatchers' && bootstrapArguments.length == 1) { + rewritten = true + super.visitInvokeDynamicInsn(indyName, indyDescriptor, + new Handle(Opcodes.H_INVOKESTATIC, bootstrap.owner, bootstrap.name, LEGACY_BSM_DESC, false)) + } else { + super.visitInvokeDynamicInsn(indyName, indyDescriptor, bootstrap, bootstrapArguments) + } + } + } + } + }, 0) + assert rewritten : 'no four-argument packedDispatchers site found to downgrade' + writer.toByteArray() + } + + private static Class parsePackedLegacyFormat() { + Map classes = withPacking { + def cu = new CompilationUnit() + cu.addSource('Host.groovy', SRC) + cu.compile(Phases.CLASS_GENERATION) + cu.classes.collectEntries { [it.name, it.bytes] } + } + def loader = new GroovyClassLoader() + Class host = null + classes.each { name, bytes -> + def clazz = loader.defineClass(name, name == 'Host' ? toLegacyFormat(bytes) : bytes) + if (name == 'Host') host = clazz + } + assert host.declaredMethods.any { it.name == '$packedDispatch$' } : 'packing did not engage' + host + } + + @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) + } + + @Test + void 'class files from earlier snapshots link through the legacy three-argument bootstrap'() { + assertEquals(['42', '42', '42', '[2, 3, 4]'], parsePackedLegacyFormat().run()) + } + + @Test + void 'undeclared checked exceptions propagate unchanged through the legacy bootstrap'() { + def thrown = assertThrows(IOException) { parsePackedLegacyFormat().boom() } + assertEquals('checked, undeclared', thrown.message) + } +}