Skip to content

Commit abbffb7

Browse files
paulk-asertpaulk-asert
committed
GROOVY-11955: Provide a size guard for flattening of chained method calls
1 parent b1fc235 commit abbffb7

2 files changed

Lines changed: 218 additions & 4 deletions

File tree

‎src/main/java/org/codehaus/groovy/classgen/asm/indy/InvokeDynamicWriter.java‎

Lines changed: 31 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -124,12 +124,40 @@ private String prepareIndyCall(final Expression receiver, final boolean implicit
124124
return "(" + getTypeDescription(operandStack.getTopOperand());
125125
}
126126

127+
// GROOVY-11955: chains shorter than this go through the normal recursive
128+
// visitor, which lets InvocationWriter emit direct invokevirtual when a
129+
// method target is resolved. Only when the chain is deep enough to risk
130+
// JVM stack overflow do we switch to the iterative indy path.
131+
private static final int CHAIN_FLATTEN_THRESHOLD = 64;
132+
127133
/**
128-
* Visits receiver expression, using iterative approach for method call chains.
129-
* GROOVY-7785: Flattens deep recursive AST structures to avoid stack overflow.
134+
* Visits receiver expression. For short chains, delegates to the normal
135+
* recursive visitor to preserve compact direct-invoke bytecode. For long
136+
* chains, flattens iteratively to avoid StackOverflowError (GROOVY-7785).
130137
*/
131138
private void visitReceiverOfMethodCall(final Expression receiver) {
132-
// Collect chain of simple method calls that can use indy optimization
139+
AsmClassGenerator acg = controller.getAcg();
140+
141+
// Fast probe: measure chain depth without allocating the deque.
142+
int depth = 0;
143+
Expression probe = receiver;
144+
while (probe instanceof MethodCallExpression mce
145+
&& !mce.isSpreadSafe() && !mce.isImplicitThis()
146+
&& !isSuperExpression(mce.getObjectExpression())
147+
&& !isThisExpression(mce.getObjectExpression())) {
148+
String name = getMethodName(mce.getMethod());
149+
if (name == null || "call".equals(name)) break;
150+
depth++;
151+
if (depth >= CHAIN_FLATTEN_THRESHOLD) break;
152+
probe = mce.getObjectExpression();
153+
}
154+
155+
if (depth < CHAIN_FLATTEN_THRESHOLD) {
156+
receiver.visit(acg);
157+
return;
158+
}
159+
160+
// Deep chain: flatten iteratively to avoid visitor recursion overflow.
133161
Deque<MethodCallExpression> chain = new ArrayDeque<>();
134162
Expression current = receiver;
135163
while (current instanceof MethodCallExpression) {
@@ -143,7 +171,6 @@ private void visitReceiverOfMethodCall(final Expression receiver) {
143171
current = mce.getObjectExpression();
144172
}
145173

146-
AsmClassGenerator acg = controller.getAcg();
147174
current.visit(acg);
148175

149176
while (!chain.isEmpty()) {

‎src/test/groovy/bugs/Groovy11955.groovy‎

Lines changed: 187 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,187 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one
3+
* or more contributor license agreements. See the NOTICE file
4+
* distributed with this work for additional information
5+
* regarding copyright ownership. The ASF licenses this file
6+
* to you under the Apache License, Version 2.0 (the
7+
* "License"); you may not use this file except in compliance
8+
* with the License. You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing,
13+
* software distributed under the License is distributed on an
14+
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
15+
* KIND, either express or implied. See the License for the
16+
* specific language governing permissions and limitations
17+
* under the License.
18+
*/
19+
package bugs
20+
21+
import org.codehaus.groovy.ast.ClassCodeVisitorSupport
22+
import org.codehaus.groovy.ast.ClassNode
23+
import org.codehaus.groovy.ast.MethodNode
24+
import org.codehaus.groovy.ast.expr.MethodCallExpression
25+
import org.codehaus.groovy.classgen.GeneratorContext
26+
import org.codehaus.groovy.control.CompilationUnit
27+
import org.codehaus.groovy.control.CompilePhase
28+
import org.codehaus.groovy.control.CompilerConfiguration
29+
import org.codehaus.groovy.control.Phases
30+
import org.codehaus.groovy.control.SourceUnit
31+
import org.codehaus.groovy.control.customizers.CompilationCustomizer
32+
import org.codehaus.groovy.tools.GroovyClass
33+
import org.junit.jupiter.api.Test
34+
import org.objectweb.asm.ClassReader
35+
import org.objectweb.asm.ClassVisitor
36+
import org.objectweb.asm.Handle
37+
import org.objectweb.asm.MethodVisitor
38+
import org.objectweb.asm.Opcodes
39+
40+
import static org.junit.jupiter.api.Assertions.assertTrue
41+
42+
/**
43+
* Boundary test for GROOVY-11955 — routing shape of receiver-chain method calls.
44+
*
45+
* <p>This is NOT a language requirement. The JVM's 64KB method-size limit caps
46+
* any large generated method and Spock's {@code verifyAll} will eventually hit
47+
* that ceiling no matter what we do. What this test guards is the <em>routing
48+
* decision</em> that determines how quickly that ceiling is reached: when an
49+
* AST transform pre-resolves a method call's target (Spock's transform does
50+
* this for every {@code ValueRecorder.record} call), the indy writer should
51+
* emit a direct {@code invokevirtual} for short receiver chains, not route
52+
* through an {@code invokedynamic} callsite. Routing through indy inflates
53+
* per-call bytecode by ~2 bytes and is measurably slower at runtime (~40% on
54+
* the original Spock reproducer after JIT warmup).
55+
*
56+
* <p>If this test fails, the fix is <em>not</em> to bump the threshold. Think
57+
* about whether the routing change is deliberate, re-measure the Spock
58+
* {@code verifyAll} cliff (the original regression shifted it from 596 to
59+
* 586 asserts at 500 assertions tested), and update the comment with the new
60+
* rationale.
61+
*
62+
* <p>See also {@code InvokeDynamicWriter.CHAIN_FLATTEN_THRESHOLD} and the
63+
* companion GROOVY-7785 test which exercises the iterative/flatten path.
64+
*/
65+
final class Groovy11955 {
66+
67+
/**
68+
* Fixture: {@code endObj()} returns {@code Object} so that a trailing
69+
* property access on the chain result forces the *outer* expression
70+
* through {@code InvokeDynamicWriter.prepareIndyCall} (the code path the
71+
* patch modifies). Without that, {@code InvocationWriter.makeDirectCall}
72+
* short-circuits the whole chain onto invokevirtual before the indy
73+
* writer ever sees it.
74+
*/
75+
private static final String HELPER_SRC = '''
76+
class Helper {
77+
Helper step() { this }
78+
Object endObj() { [field: 'x'] }
79+
}
80+
'''
81+
82+
/**
83+
* Mimics what Spock's AST transform does: for every MCE whose method name
84+
* matches a Helper method, set {@code methodTarget} so
85+
* {@code InvocationWriter.makeDirectCall} takes the invokevirtual fast
86+
* path when the recursive visitor sees it.
87+
*/
88+
static class ResolveHelperTargets extends CompilationCustomizer {
89+
ResolveHelperTargets() { super(CompilePhase.SEMANTIC_ANALYSIS) }
90+
91+
@Override
92+
void call(SourceUnit source, GeneratorContext context, ClassNode classNode) {
93+
ClassNode helperCN = source.AST.classes.find { it.nameWithoutPackage == 'Helper' }
94+
if (!helperCN) return
95+
classNode.visitContents(new ClassCodeVisitorSupport() {
96+
@Override protected SourceUnit getSourceUnit() { source }
97+
98+
@Override
99+
void visitMethodCallExpression(MethodCallExpression mce) {
100+
super.visitMethodCallExpression(mce)
101+
if (mce.methodTarget != null) return
102+
String name = mce.method.text
103+
List<MethodNode> candidates = helperCN.getMethods(name)
104+
if (candidates.size() == 1) mce.methodTarget = candidates[0]
105+
}
106+
})
107+
}
108+
}
109+
110+
/** Compile Helper + the given script body and return the script class bytes. */
111+
private static byte[] compile(String scriptBody) {
112+
def config = new CompilerConfiguration()
113+
config.addCompilationCustomizers(new ResolveHelperTargets())
114+
def cu = new CompilationUnit(config)
115+
cu.addSource('Groovy11955Src.groovy', HELPER_SRC + scriptBody)
116+
cu.compile(Phases.CLASS_GENERATION)
117+
GroovyClass scriptClass = cu.classes.find { it.name == 'Groovy11955Src' }
118+
assert scriptClass : "no script class; got: ${cu.classes*.name}"
119+
return scriptClass.bytes
120+
}
121+
122+
/**
123+
* Count method-call instruction shapes in {@code run()} of the compiled
124+
* class, filtered to those targeting Helper. The iterative chain-flatten
125+
* indy calls all carry call site name {@code "invoke"} and the target
126+
* method name as the first BSM arg.
127+
*/
128+
private static Map<String, Integer> helperCalls(byte[] classBytes) {
129+
def out = [invokevirtual: 0, invokedynamic: 0]
130+
new ClassReader(classBytes).accept(new ClassVisitor(Opcodes.ASM9) {
131+
@Override
132+
MethodVisitor visitMethod(int access, String name, String desc, String sig, String[] exs) {
133+
if (name != 'run') return null
134+
return new MethodVisitor(Opcodes.ASM9) {
135+
@Override
136+
void visitMethodInsn(int op, String owner, String n, String d, boolean itf) {
137+
if (op == Opcodes.INVOKEVIRTUAL && owner == 'Helper') {
138+
out.invokevirtual++
139+
}
140+
}
141+
@Override
142+
void visitInvokeDynamicInsn(String n, String d, Handle bsm, Object... args) {
143+
if (n == 'invoke' && args.length > 0) {
144+
String mname = String.valueOf(args[0])
145+
if (mname == 'step' || mname == 'endObj') out.invokedynamic++
146+
}
147+
}
148+
}
149+
}
150+
}, 0)
151+
return out
152+
}
153+
154+
// -- Tests ---------------------------------------------------------------
155+
156+
@Test
157+
void shortChainKeepsChainReceiverCallsOnInvokeVirtual() {
158+
// Chain depth 4 as the receiver of a property access (well below
159+
// CHAIN_FLATTEN_THRESHOLD). With pre-resolved targets, all four
160+
// Helper calls must emit invokevirtual — not invokedynamic.
161+
def src = '''
162+
def h = new Helper()
163+
h.step().step().step().endObj().field
164+
'''
165+
def c = helperCalls(compile(src))
166+
167+
assertTrue(c.invokevirtual >= 4,
168+
"expected >= 4 Helper invokevirtual calls, got ${c}")
169+
assertTrue(c.invokedynamic == 0,
170+
"expected 0 Helper invokedynamic calls for short chain, got ${c}")
171+
}
172+
173+
@Test
174+
void longChainFlattensToInvokeDynamic() {
175+
// Depth > CHAIN_FLATTEN_THRESHOLD (64): iterative path engages;
176+
// every Helper call in the receiver chain is emitted as an indy
177+
// callsite rather than invokevirtual.
178+
def sb = new StringBuilder('def h = new Helper(); h')
179+
80.times { sb << '.step()' }
180+
sb << '.endObj().field'
181+
def c = helperCalls(compile(sb.toString()))
182+
183+
// Exact count isn't the point — presence of many indy calls is.
184+
assertTrue(c.invokedynamic >= 60,
185+
"expected flatten path to emit many Helper indy calls, got ${c}")
186+
}
187+
}

0 commit comments

Comments
 (0)