Copilot commented on code in PR #2709: URL: https://github.com/apache/groovy/pull/2709#discussion_r3576679993
########## subprojects/performance/src/jmh/groovy/org/apache/groovy/bench/ClosurePackBench.java: ########## @@ -0,0 +1,154 @@ +/* + * 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.apache.groovy.bench; + +import groovy.lang.GroovyClassLoader; +import org.openjdk.jmh.annotations.Benchmark; +import org.openjdk.jmh.annotations.BenchmarkMode; +import org.openjdk.jmh.annotations.Fork; +import org.openjdk.jmh.annotations.Level; +import org.openjdk.jmh.annotations.Measurement; +import org.openjdk.jmh.annotations.Mode; +import org.openjdk.jmh.annotations.OutputTimeUnit; +import org.openjdk.jmh.annotations.Param; +import org.openjdk.jmh.annotations.Scope; +import org.openjdk.jmh.annotations.Setup; +import org.openjdk.jmh.annotations.State; +import org.openjdk.jmh.annotations.Warmup; + +import java.util.ArrayList; +import java.util.List; +import java.util.concurrent.TimeUnit; + +/** + * Steady-state closure invocation under closure packing (GROOVY-12151, + * {@code groovy.target.closure.pack}) versus generated closure classes, for the + * two hot closure shapes: a non-capturing {@code collect} body and a + * written-capture {@code each} accumulator. + * <p> + * The flag is read at closure-emission time, so {@link #setup} compiles the + * fixture classes with a fresh {@link GroovyClassLoader} after setting it from + * the {@code pack} param — JMH runs each param value in its own forks, so the + * two states never share a JVM or its profile. A vacuity guard asserts the flag + * really flipped the emission (no {@code _closure} classes when packing, some + * when not). + * <p> + * Each shape runs twice: {@code mono} always invokes one fixture class, the + * best case for JIT inlining; {@code mega} rotates over {@link #CLASSES} + * fixture classes, making the shared {@code PackedClosure} adapter's dispatcher + * call site megamorphic (each class has its own hidden dispatcher class) — the + * realistic many-classes regime, and the honest comparison against generated + * closure classes, whose {@code call} sites in DGM are equally megamorphic. + * Single-class microbenches overstate packed dispatch costs (a deep monomorphic + * inline of the shared adapter trips the JIT's {@code InlineSmallCode} + * heuristic); this pair brackets the real-world range instead. + */ +@Warmup(iterations = 5, time = 1) +@Measurement(iterations = 5, time = 1) +@Fork(2) +@BenchmarkMode(Mode.Throughput) +@OutputTimeUnit(TimeUnit.MILLISECONDS) +@State(Scope.Benchmark) +public class ClosurePackBench { + + /** Implemented by every compiled fixture class, so invocation needs no reflection. */ + public interface PackWorkload { + Object squared(List<Integer> xs); + int sum(List<Integer> xs); + } + + private static final int CLASSES = 16; // power of two: mega rotation uses a mask + + /** Compile-time flag: {@code true} packs eligible closures, {@code false} = closure classes. */ + @Param({"false", "true"}) + public String pack; + + private final PackWorkload[] fixtures = new PackWorkload[CLASSES]; + private List<Integer> xs; + private int idx; + + @Setup(Level.Trial) + public void setup() throws Exception { + System.setProperty("groovy.target.closure.pack", pack); + try (GroovyClassLoader loader = new GroovyClassLoader(getClass().getClassLoader())) { + for (int i = 0; i < CLASSES; i += 1) { + String source = + "@groovy.transform.CompileStatic\n" + + "class Fix" + i + " implements org.apache.groovy.bench.ClosurePackBench.PackWorkload {\n" + + " Object squared(List<Integer> xs) { xs.collect { it * it } }\n" + + " int sum(List<Integer> xs) { int s = 0; xs.each { Integer x -> s += x }; s }\n" + + "}\n"; + Class<?> c = loader.parseClass(source, "Fix" + i + ".groovy"); + fixtures[i] = (PackWorkload) c.getDeclaredConstructor().newInstance(); + } + // vacuity guard: the flag must actually gate the emission + boolean closureClasses = false; + for (Class<?> c : loader.getLoadedClasses()) { + if (c.getName().contains("_closure")) closureClasses = true; + } + if (Boolean.parseBoolean(pack) == closureClasses) { + throw new IllegalStateException("closure.pack=" + pack + " but closure classes " + + (closureClasses ? "were" : "were not") + " generated"); + } + } + xs = new ArrayList<>(); Review Comment: setup() sets the global system property groovy.target.closure.pack but never restores it. In a single JMH fork, benchmarks run in the same JVM, so this can leak into other benchmarks (and into any Groovy compilation they do), skewing results. ########## src/main/java/org/codehaus/groovy/classgen/asm/ClosureWriter.java: ########## @@ -158,6 +237,644 @@ public void writeClosure(final ClosureExpression expression) { controller.getOperandStack().replace(ClassHelper.CLOSURE_TYPE, localVariableParams.length); } + /** + * GEP-27 capability analysis: choose the compilation strategy for a closure literal. + * <p> + * A closure is packed (S1, {@link PackStrategy#PACKED_ADAPTER}) when it is <em>triggered</em> — + * either the enclosing scope opts in via {@code @PackedClosures} (the dynamic trust path), or the + * capability analysis <em>proves</em> it delegate-independent under {@code @CompileStatic} (the + * automatic path, see {@link #isDelegateIndependent}) — and it is in a packable position: + * it needs no real {@code Closure} semantics we cannot reproduce ({@link #isPackable}), it is + * written directly in a method rather than nested in another closure (owner-retargeting for + * nested closures is later work), and it does not visibly escape its method. Otherwise it stays a + * full generated class (S0, {@link PackStrategy#FULL_CLASS}) exactly as today. + */ + private PackStrategy chooseStrategy(final ClosureExpression expression) { + if (!isPackable(expression)) return PackStrategy.FULL_CLASS; + // The most-specific @PackedClosures mode in scope (method wins over class), or null if not + // annotated. DISABLED is a fine-grained opt-out that beats an enclosing opt-in AND the flag. + PackMode mode = annotatedMode(); + if (mode == PackMode.DISABLED) return PackStrategy.FULL_CLASS; + boolean annotated = (mode != null); // LENIENT/WARN/STRICT all opt in (DISABLED handled above) + if (!isPackTriggered(expression, annotated)) return PackStrategy.FULL_CLASS; + if (controller.isInGeneratedFunction()) return PackStrategy.FULL_CLASS; + if (escapesEnclosingMethod(expression)) return PackStrategy.FULL_CLASS; + return PackStrategy.PACKED_ADAPTER; + } + + /** + * Whether a packable, non-escaping closure literal should actually be packed. Dynamic compilation + * has no proof of delegate-independence, so the only trigger is the {@code @PackedClosures} trust + * assertion; {@code StaticTypesClosureWriter} overrides this to add the + * {@code groovy.target.closure.pack} flag and the delegate-independence proof. + * + * @param annotated whether a non-DISABLED {@code @PackedClosures} is in scope + */ + protected boolean isPackTriggered(final ClosureExpression expression, final boolean annotated) { + return annotated; + } + + /** + * A trigger-specific decline reason for {@link #declineReason}, or {@code null} if the trigger did + * not decline this closure. Only the static writer has one (a delegate-resolved body); the dynamic + * path trusts the annotation and never declines on trigger grounds. + */ + protected String triggerDeclineReason(final ClosureExpression expression) { + return null; + } + + /** Whether the hoisted body is compiled statically (true only for {@code @CompileStatic}). */ + protected boolean compilesHoistedBodyStatically() { + return false; + } + + /** + * Whether the packed adapter installs the runtime delegate guard. The dynamic trust path needs it + * (an unverifiable assertion must fail fast on misuse); the static writer proved independence, so + * it overrides this to {@code false} and a caller-set delegate is then stored and ignored. + */ + protected boolean packedClosureUsesDelegateGuard() { + return true; + } + + /** The closure's inferred return type, normalised the same way {@code createClosureClass} does for doCall. */ + private static ClassNode inferredClosureReturnType(final ClosureExpression expression) { + ClassNode returnType = expression.getNodeMetaData(INFERRED_RETURN_TYPE); + if (returnType == null) returnType = ClassHelper.OBJECT_TYPE; + else if (returnType.isPrimaryClassNode()) returnType = returnType.getPlainNodeReference(); + else if (ClassHelper.isPrimitiveType(returnType)) returnType = ClassHelper.getWrapper(returnType); + else if (GenericsUtils.hasUnresolvedGenerics(returnType)) returnType = GenericsUtils.nonGeneric(returnType); + return returnType; + } + + /** + * The erasure of a captured variable's type, safe to declare on the hoisted method: generic + * placeholders are replaced by their bound (the hoisted method does not declare the enclosing + * method's type variables) and type arguments are dropped ({@code List<T>} → {@code List}). + */ + private static ClassNode erasedType(final ClassNode type) { + ClassNode t = type; + if (t == null) return ClassHelper.OBJECT_TYPE; + if (t.isGenericsPlaceHolder()) t = t.redirect(); + return t.getPlainNodeReference(); + } + + /** + * Reports a top-level closure that was NOT packed, per the {@code mode} of the enclosing + * {@code @PackedClosures} annotation (WARN => compiler warning, STRICT => compiler error). Only the + * annotation opts in; the automatic {@code groovy.target.closure.pack} path is always lenient. A + * nested closure is a structural decline (its enclosing context is a generated function, not the + * owner class), so it is not reported. + */ + private void reportUnpacked(final ClosureExpression expression) { + if (controller.isInGeneratedFunction()) return; + PackMode mode = annotatedMode(); + // LENIENT/DISABLED are both silent -- DISABLED asked NOT to pack, so a decline is expected + if (mode == null || mode == PackMode.LENIENT || mode == PackMode.DISABLED) return; + String msg = "@PackedClosures: closure was not packed -- " + declineReason(expression); + if (mode == PackMode.STRICT) { + controller.getSourceUnit().getErrorCollector() + .addErrorAndContinue(msg, expression, controller.getSourceUnit()); + } else { + // WARN is opt-in, so emit at LIKELY_ERRORS to show at the default warning level (the plain + // addWarning(text, node) uses POSSIBLE_ERRORS, which the default level suppresses). groovyc + // surfaces collected warnings on success since GROOVY-12132; Gradle/IDEs already do. + controller.getSourceUnit().addWarning(WarningMessage.LIKELY_ERRORS, msg, expression); + } + } + + /** The most specific {@code @PackedClosures} mode in scope (method wins over class), or null if not annotated. */ + private PackMode annotatedMode() { + MethodNode m = controller.getMethodNode(); + PackMode mode = (m != null) ? packModeOf(m.getAnnotations()) : null; + if (mode != null) return mode; + for (ClassNode c = controller.getClassNode(); c != null; c = c.getOuterClass()) { + mode = packModeOf(c.getAnnotations()); + if (mode != null) return mode; + } + return null; + } + + /** The mode of a {@code @PackedClosures} in the given set (LENIENT if present without an explicit mode), or null. */ + private static PackMode packModeOf(final List<AnnotationNode> annotations) { + for (AnnotationNode a : annotations) { + if (a.getClassNode() != null && "groovy.transform.PackedClosures".equals(a.getClassNode().getName())) { + Expression member = a.getMember("mode"); + if (member instanceof PropertyExpression) { + try { + return PackMode.valueOf(((PropertyExpression) member).getPropertyAsString()); + } catch (IllegalArgumentException ignore) { + // malformed member; treat as the default + } + } + return PackMode.LENIENT; + } + } + return null; + } + + /** Why {@link #chooseStrategy} declined this (packable, top-level) closure -- the message WARN/STRICT report. */ + private String declineReason(final ClosureExpression expression) { + if (!isPackable(expression)) { + return "it needs a real Closure (uses owner/delegate/thisObject/resolveStrategy/super or metaClass, " + + "has default parameter values, or contains an anonymous inner class)"; + } + String triggerReason = triggerDeclineReason(expression); + if (triggerReason != null) return triggerReason; + if (escapesEnclosingMethod(expression)) { + return "it escapes its method (returned, stored to a field/property/index, appended, or in a collection literal)"; + } + return "it is not eligible for packing in this scope"; + } + + /** + * Conservative compile-time escape gate. A packed closure is bound to the owner and backed by a + * shared adapter that fails fast if a delegate is later set on it. To avoid that surprise for the + * clearest cases, decline (keep a real closure class for) a closure literal that visibly escapes + * the method it is written in: stored into a field/property/index (e.g. {@code attrs.optionValue = + * { ... }}), returned, appended with {@code <<}, or placed in a list/map literal. Transitive escapes + * (via a local that is later returned) are intentionally not chased here; the runtime guard remains + * the backstop for those. + */ + private boolean escapesEnclosingMethod(final ClosureExpression expression) { + MethodNode m = controller.getMethodNode(); + if (m == null || m.getCode() == null) return false; + return escapingClosuresByMethod + .computeIfAbsent(m, k -> collectEscapingClosures(k.getCode())) + .contains(expression); + } + + private static Set<ClosureExpression> collectEscapingClosures(final Statement code) { + Set<ClosureExpression> escaping = Collections.newSetFromMap(new IdentityHashMap<>()); + code.visit(new CodeVisitorSupport() { + private void mark(final Expression e) { + if (e instanceof ClosureExpression) escaping.add((ClosureExpression) e); + } + @Override public void visitReturnStatement(final ReturnStatement statement) { + mark(statement.getExpression()); + super.visitReturnStatement(statement); + } + @Override public void visitBinaryExpression(final BinaryExpression be) { + int op = be.getOperation().getType(); + if (op == Types.EQUAL && !isLocalTarget(be.getLeftExpression())) { + mark(be.getRightExpression()); // assigned to a field/property/index + } else if (op == Types.LEFT_SHIFT) { + mark(be.getRightExpression()); // appended, e.g. list << { ... } + } + super.visitBinaryExpression(be); + } + @Override public void visitListExpression(final ListExpression le) { + le.getExpressions().forEach(this::mark); + super.visitListExpression(le); + } + @Override public void visitMapEntryExpression(final MapEntryExpression me) { + mark(me.getValueExpression()); + super.visitMapEntryExpression(me); + } + }); + return escaping; + } + + /** True when an assignment target is a plain local variable (not a field/property that escapes). */ + private static boolean isLocalTarget(final Expression lhs) { + if (!(lhs instanceof VariableExpression)) return false; // property/field/index target + Variable v = ((VariableExpression) lhs).getAccessedVariable(); + return !(v instanceof FieldNode) && !(v instanceof PropertyNode); + } + + /** + * Spike packability gate: pack read-only closures (including nested and captures). Captured-variable + * writes are supported via Reference threading for a flat closure only (declined if the body also + * contains a nested closure, or uses an unsupported compound-assignment operator). Anything needing a + * real Closure — delegate/owner/thisObject/resolveStrategy or {@code super} — is declined. + */ + private static boolean isPackable(final ClosureExpression expression) { + Statement code = expression.getCode(); + if (code == null) return false; + // Default parameter values generate arity-overloaded doCall methods on a closure class; + // the single hoisted method cannot reproduce that dispatch, so decline. + if (expression.isParameterSpecified()) { + for (Parameter p : expression.getParameters()) { + if (p.hasInitialExpression()) return false; + } + } + boolean[] ok = {true}; + code.visit(new CodeVisitorSupport() { + @Override public void visitClosureExpression(final ClosureExpression e) { + // A nested closure constructed inside the hoisted method gets the enclosing INSTANCE + // as its owner instead of the (packed) outer closure object. That is observationally + // equivalent for owner-chain resolution, but not for a nested body that names + // owner/delegate/thisObject explicitly -- so the outer packs only if every nested + // closure is itself packable. + if (!isPackable(e)) ok[0] = false; + } + @Override public void visitVariableExpression(final VariableExpression ve) { + if (ve.isSuperExpression() || FORBIDDEN_CLOSURE_NAMES.contains(ve.getName())) ok[0] = false; + } + @Override public void visitMethodCallExpression(final MethodCallExpression call) { + // implicit-this accessor/mutator forms of the same pseudo-properties, e.g. getDelegate() + if (call.isImplicitThis() && FORBIDDEN_CLOSURE_CALLS.contains(call.getMethodAsString())) ok[0] = false; + super.visitMethodCallExpression(call); + } + @Override public void visitConstructorCallExpression(final ConstructorCallExpression cce) { + // an anonymous inner class in the body is generated against the enclosing closure + // class (its outer instance); hoisting would hand it the wrong enclosing instance + if (cce.isUsingAnonymousInnerClass()) ok[0] = false; + super.visitConstructorCallExpression(cce); + } + }); + // All write forms to captured variables are supported: the shared Reference is passed as a + // holder parameter, so the ASM generator emits the implicit get()/set() exactly as it does + // for a closure class -- no operator restrictions. + return ok[0]; + } + + private static Set<String> capturedNames(final ClosureExpression expression) { + Set<String> captured = new HashSet<>(); + VariableScope vs = expression.getVariableScope(); + if (vs != null) { + for (Iterator<Variable> it = vs.getReferencedLocalVariablesIterator(); it.hasNext(); ) { + captured.add(it.next().getName()); + } + } + return captured; + } + + /** Captured variables that are written (reassigned) in the body — these need Reference threading. */ + private Set<String> writtenCaptureNames(final ClosureExpression expression) { + Set<String> captured = capturedNames(expression); + Set<String> written = new HashSet<>(collectWrittenNames(expression.getCode())); + MethodNode m = controller.getMethodNode(); + if (m != null && m.getCode() != null) { + written.addAll(writtenNamesByMethod.computeIfAbsent(m, k -> collectWrittenNames(k.getCode()))); + } + written.retainAll(captured); + return written; + } + + /** Names assigned or incremented in the given code (declarations with initializers excluded). */ + private static Set<String> collectWrittenNames(final Statement code) { + Set<String> written = new HashSet<>(); + if (code == null) return written; + code.visit(new CodeVisitorSupport() { // descends into nested closures + @Override public void visitBinaryExpression(final BinaryExpression be) { + if (Types.isAssignment(be.getOperation().getType()) + && !(be instanceof org.codehaus.groovy.ast.expr.DeclarationExpression) + && be.getLeftExpression() instanceof VariableExpression) { + written.add(((VariableExpression) be.getLeftExpression()).getName()); + } + super.visitBinaryExpression(be); + } + @Override public void visitPostfixExpression(final PostfixExpression pe) { + recordIncrement(pe.getExpression()); + super.visitPostfixExpression(pe); + } + @Override public void visitPrefixExpression(final PrefixExpression pe) { + recordIncrement(pe.getExpression()); + super.visitPrefixExpression(pe); + } + private void recordIncrement(final Expression operand) { + if (operand instanceof VariableExpression) { + written.add(((VariableExpression) operand).getName()); + } + } + }); + return written; + } + + // NB: metaClass resolves against the closure object ITSELF (per-closure-class identity), which + // the shared adapter cannot reproduce, so it declines like the other closure pseudo-properties. + private static final Set<String> FORBIDDEN_CLOSURE_NAMES = + new HashSet<>(java.util.Arrays.asList("owner", "delegate", "thisObject", "directive", + "resolveStrategy", "metaClass")); + + /** Accessor/mutator forms of the same pseudo-properties, called with implicit this. */ + private static final Set<String> FORBIDDEN_CLOSURE_CALLS = + new HashSet<>(java.util.Arrays.asList("getOwner", "getDelegate", "getThisObject", "getDirective", + "getResolveStrategy", "setDelegate", "setDirective", "setResolveStrategy", + "getMaximumNumberOfParameters", "getParameterTypes", "getMetaClass", "setMetaClass", + "invokeMethod", "getProperty", "setProperty")); + + /** + * Rebinds captured-variable references in the moved body to the hoisted method's parameters, + * matched by the accessed {@link Variable}'s <em>identity</em> rather than by name. Groovy forbids + * shadowing an in-scope local/parameter, so a name match would be correct today; keying on identity + * keeps it correct even if that scoping rule ever relaxed, and never rewrites an unrelated binding + * that happens to share a captured name. + */ + private static void rebindCaptured(final Statement body, final Map<Variable, Parameter> capturedParams) { + body.visit(new CodeVisitorSupport() { + @Override public void visitVariableExpression(final VariableExpression ve) { + Parameter p = capturedParams.get(ve.getAccessedVariable()); + if (p != null) { + ve.setAccessedVariable(p); + // read-only captures become plain by-value params; written captures stay + // closure-shared so the ASM generator routes them through the holder + ve.setClosureSharedVariable(p.isClosureSharedVariable()); + } + } + }); + } + + /** + * Emits a packed closure: hoists the body to a synthetic method on the enclosing class (captured + * variables become leading, by-value parameters) and constructs a shared {@code PackedClosure} + * adapter that dispatches to it — no inner class. + */ + private void writePackedClosure(final ClosureExpression expression) { + ClassNode enclosing = controller.getClassNode(); + MethodVisitor mv = controller.getMethodVisitor(); + AsmClassGenerator acg = controller.getAcg(); + OperandStack os = controller.getOperandStack(); + + List<String> captured = new ArrayList<>(); + Map<String, ClassNode> capturedTypes = new HashMap<>(); + Map<String, Variable> capturedVars = new HashMap<>(); // name -> the original captured variable + VariableScope vs = expression.getVariableScope(); + if (vs != null) { + for (Iterator<Variable> it = vs.getReferencedLocalVariablesIterator(); it.hasNext(); ) { + Variable v = it.next(); + captured.add(v.getName()); + capturedTypes.put(v.getName(), erasedType(v.getOriginType())); + capturedVars.put(v.getName(), v); + } + } + + Parameter[] closureParams = expression.isParameterSpecified() + ? expression.getParameters() + : new Parameter[]{new Parameter(ClassHelper.OBJECT_TYPE, "it")}; + int arity = closureParams.length; + + Set<String> written = writtenCaptureNames(expression); + Parameter[] methodParams = new Parameter[captured.size() + closureParams.length]; + Map<Variable, Parameter> capturedByVar = new IdentityHashMap<>(); // original variable -> hoisted param + boolean typedBody = compilesHoistedBodyStatically(); + for (int i = 0; i < captured.size(); i++) { + String cn = captured.get(i); + boolean isWritten = written.contains(cn); + Parameter p; + if (isWritten) { + // A written capture arrives as the SHARED groovy.lang.Reference itself: declare the + // parameter exactly like a closure-class constructor does (originType = value type, + // descriptor type = Reference, closure-shared + use-existing-reference), so + // CompileStack.init registers it as a holder and the ASM generator emits the implicit + // get()/set() on every read/write -- no AST rewriting, and the untouched body keeps + // the STC metadata that lets it compile statically. + ClassNode vt = capturedTypes.get(cn); + if (ClassHelper.isPrimitiveType(vt)) vt = ClassHelper.getWrapper(vt); + p = new Parameter(vt, cn); + p.setType(ClassHelper.makeReference()); + p.setClosureSharedVariable(true); + p.putNodeMetaData(UseExistingReference.class, Boolean.TRUE); + // the static writer's type chooser must see the VALUE type: the holder load already + // dereferences, so resolving this variable as Reference would add a bogus checkcast + p.putNodeMetaData(org.codehaus.groovy.transform.stc.StaticTypesMarker.INFERRED_TYPE, vt); + } else { + // For a statically-compiled body, type the read-only capture from its declared type so + // the loads match the STC metadata already on the body expressions (otherwise the + // static writer falls back to dynamic dispatch on the type mismatch). + p = new Parameter(typedBody ? capturedTypes.get(cn) : ClassHelper.OBJECT_TYPE, cn); + } + methodParams[i] = p; + capturedByVar.put(capturedVars.get(cn), p); + } + System.arraycopy(closureParams, 0, methodParams, captured.size(), closureParams.length); + + Statement body = expression.getCode(); + rebindCaptured(body, capturedByVar); // read-only -> plain param; written -> holder param + + // In a static method there is no `this`: the owner is the enclosing class and the hoisted + // method must be static (loaded/dispatched against the class, not local slot 0). + boolean staticContext = controller.isStaticMethod(); + List<MethodNode> targets = packedTargets(enclosing); + int id = targets.size(); // ids are dense per class: the dispatcher emits a tableswitch over them + String name = PACKED_METHOD_PREFIX + id; + // PRIVATE, not public: the body is reached only reflectively via PackedClosure.invokeHoisted, and + // a private method is invoked exactly (no virtual dispatch), so an inherited packed closure never + // dispatches to a same-named hoisted method a subclass happens to declare (GROOVY-12151 review). Review Comment: This comment says the hoisted body is reached “reflectively via PackedClosure.invokeHoisted”, but the implementation dispatches via the generated $packedDispatch$ tableswitch and INVOKESPECIAL/INVOKESTATIC. Keeping the comment accurate matters for maintainers reasoning about dispatch/inlining. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
