Skip to content

Commit 249491a

Browse files
blackdragdaniellansun
authored andcommitted
GROOVY-11842: avoid IndyInterface polluting trace (#2369)
* ensure the first call of invokedynamic is not polluting the stack trace * IndyUsageTest will no longer work since there will be no IndyInterface lines in the trace anymore * adding test for deprecated methods to help backwards compatibility * cleanup in IndyInterface (cherry picked from commit 53e1b68)
1 parent 0d18ea0 commit 249491a

File tree

3 files changed

+165
-59
lines changed

3 files changed

+165
-59
lines changed

src/main/java/org/codehaus/groovy/vmplugin/v8/IndyInterface.java

Lines changed: 60 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -153,30 +153,31 @@ public int getOrderNumber() {
153153
public static final MethodHandles.Lookup LOOKUP = MethodHandles.lookup();
154154

155155
/**
156-
* handle for the fromCache method
156+
* handle for the fromCacheHandle method
157157
*/
158-
private static final MethodHandle FROM_CACHE_METHOD;
158+
private static final MethodHandle FROM_CACHE_HANDLE_METHOD;
159159

160160
/**
161-
* handle for the selectMethod method
161+
* handle for the selectMethodHandle method
162162
*/
163-
private static final MethodHandle SELECT_METHOD;
163+
private static final MethodHandle SELECT_METHOD_HANDLE_METHOD;
164+
165+
/**
166+
* shared invoker for cached method handles
167+
*/
168+
private static final MethodHandle CACHED_INVOKER;
164169

165170
static {
166171

167172
try {
168-
MethodType mt = MethodType.methodType(Object.class, CacheableCallSite.class, Class.class, String.class, int.class, Boolean.class, Boolean.class, Boolean.class, Object.class, Object[].class);
169-
FROM_CACHE_METHOD = LOOKUP.findStatic(IndyInterface.class, "fromCache", mt);
173+
MethodType handleMt = MethodType.methodType(MethodHandle.class, CacheableCallSite.class, Class.class, String.class, int.class, Boolean.class, Boolean.class, Boolean.class, Object.class, Object[].class);
174+
FROM_CACHE_HANDLE_METHOD = LOOKUP.findStatic(IndyInterface.class, "fromCacheHandle", handleMt);
175+
SELECT_METHOD_HANDLE_METHOD = LOOKUP.findStatic(IndyInterface.class, "selectMethodHandle", handleMt);
170176
} catch (Exception e) {
171177
throw new GroovyBugError(e);
172178
}
173179

174-
try {
175-
MethodType mt = MethodType.methodType(Object.class, CacheableCallSite.class, Class.class, String.class, int.class, Boolean.class, Boolean.class, Boolean.class, Object.class, Object[].class);
176-
SELECT_METHOD = LOOKUP.findStatic(IndyInterface.class, "selectMethod", mt);
177-
} catch (Exception e) {
178-
throw new GroovyBugError(e);
179-
}
180+
CACHED_INVOKER = MethodHandles.exactInvoker(MethodType.methodType(Object.class, Object[].class));
180181
}
181182

182183
protected static SwitchPoint switchPoint = new SwitchPoint();
@@ -254,19 +255,39 @@ private static CallSite realBootstrap(MethodHandles.Lookup caller, String name,
254255
* Makes a fallback method for an invalidated method selection
255256
*/
256257
protected static MethodHandle makeFallBack(MutableCallSite mc, Class<?> sender, String name, int callID, MethodType type, boolean safeNavigation, boolean thisCall, boolean spreadCall) {
257-
return make(mc, sender, name, callID, type, safeNavigation, thisCall, spreadCall, SELECT_METHOD);
258+
return makeBoothandle(mc, sender, name, callID, type, safeNavigation, thisCall, spreadCall, SELECT_METHOD_HANDLE_METHOD);
258259
}
259260

260261
/**
261262
* Makes an adapter method for method selection, i.e. get the cached methodhandle(fast path) or fallback
262263
*/
263264
private static MethodHandle makeAdapter(MutableCallSite mc, Class<?> sender, String name, int callID, MethodType type, boolean safeNavigation, boolean thisCall, boolean spreadCall) {
264-
return make(mc, sender, name, callID, type, safeNavigation, thisCall, spreadCall, FROM_CACHE_METHOD);
265+
return makeBoothandle(mc, sender, name, callID, type, safeNavigation, thisCall, spreadCall, FROM_CACHE_HANDLE_METHOD);
265266
}
266267

267-
private static MethodHandle make(MutableCallSite mc, Class<?> sender, String name, int callID, MethodType type, boolean safeNavigation, boolean thisCall, boolean spreadCall, MethodHandle originalMH) {
268-
MethodHandle mh = MethodHandles.insertArguments(originalMH, 0, mc, sender, name, callID, safeNavigation, thisCall, spreadCall, /*dummy receiver:*/ 1);
269-
return mh.asCollector(Object[].class, type.parameterCount()).asType(type);
268+
private static MethodHandle makeBoothandle(MutableCallSite mc, Class<?> sender, String name, int callID, MethodType type, boolean safeNavigation, boolean thisCall, boolean spreadCall, MethodHandle handleReturningMh) {
269+
// Step 1: bind site-constant arguments (incl dummy receiver marker)
270+
MethodHandle fromCacheBound = MethodHandles.insertArguments(
271+
handleReturningMh,
272+
0, mc, sender, name, callID,
273+
safeNavigation, thisCall, spreadCall,
274+
/*dummy receiver*/ 1
275+
);
276+
// fromCacheBound: (Object receiver, Object[] args) → MethodHandle
277+
278+
// Step 2: fold into the shared invoker (MethodHandle, Object[]) → Object
279+
MethodHandle boothandle = MethodHandles.foldArguments(
280+
CACHED_INVOKER, // (MethodHandle, Object[]) → Object
281+
fromCacheBound // (Object, Object[]) → MethodHandle
282+
);
283+
// boothandle: (Object receiver, Object[] args) → Object
284+
285+
// Step 3: adapt to callsite type: collect all arguments into Object[] and then asType
286+
boothandle = boothandle
287+
.asCollector(Object[].class, type.parameterCount())
288+
.asType(type);
289+
290+
return boothandle;
270291
}
271292

272293
private static class FallbackSupplier {
@@ -304,8 +325,18 @@ MethodHandleWrapper get() {
304325

305326
/**
306327
* Get the cached methodhandle. if the related methodhandle is not found in the inline cache, cache and return it.
328+
* @deprecated Use the new boothandle-based approach instead.
307329
*/
330+
@Deprecated
308331
public static Object fromCache(CacheableCallSite callSite, Class<?> sender, String methodName, int callID, Boolean safeNavigation, Boolean thisCall, Boolean spreadCall, Object dummyReceiver, Object[] arguments) throws Throwable {
332+
MethodHandle mh = fromCacheHandle(callSite, sender, methodName, callID, safeNavigation, thisCall, spreadCall, dummyReceiver, arguments);
333+
return mh.invokeExact(arguments);
334+
}
335+
336+
/**
337+
* Get the cached methodhandle. if the related methodhandle is not found in the inline cache, cache and return it.
338+
*/
339+
private static MethodHandle fromCacheHandle(CacheableCallSite callSite, Class<?> sender, String methodName, int callID, Boolean safeNavigation, Boolean thisCall, Boolean spreadCall, Object dummyReceiver, Object[] arguments) throws Throwable {
309340
FallbackSupplier fallbackSupplier = new FallbackSupplier(callSite, sender, methodName, callID, safeNavigation, thisCall, spreadCall, dummyReceiver, arguments);
310341

311342
MethodHandleWrapper mhw =
@@ -341,7 +372,7 @@ public static Object fromCache(CacheableCallSite callSite, Class<?> sender, Stri
341372
mhw.resetLatestHitCount();
342373
}
343374

344-
return mhw.getCachedMethodHandle().invokeExact(arguments);
375+
return mhw.getCachedMethodHandle();
345376
}
346377

347378
private static boolean bypassCache(Boolean spreadCall, Object[] arguments) {
@@ -352,8 +383,18 @@ private static boolean bypassCache(Boolean spreadCall, Object[] arguments) {
352383

353384
/**
354385
* Core method for indy method selection using runtime types.
386+
* @deprecated Use the new boothandle-based approach instead.
355387
*/
388+
@Deprecated
356389
public static Object selectMethod(CacheableCallSite callSite, Class<?> sender, String methodName, int callID, Boolean safeNavigation, Boolean thisCall, Boolean spreadCall, Object dummyReceiver, Object[] arguments) throws Throwable {
390+
MethodHandle mh = selectMethodHandle(callSite, sender, methodName, callID, safeNavigation, thisCall, spreadCall, dummyReceiver, arguments);
391+
return mh.invokeExact(arguments);
392+
}
393+
394+
/**
395+
* Core method for indy method selection using runtime types.
396+
*/
397+
private static MethodHandle selectMethodHandle(CacheableCallSite callSite, Class<?> sender, String methodName, int callID, Boolean safeNavigation, Boolean thisCall, Boolean spreadCall, Object dummyReceiver, Object[] arguments) throws Throwable {
357398
MethodHandleWrapper mhw = fallback(callSite, sender, methodName, callID, safeNavigation, thisCall, spreadCall, dummyReceiver, arguments);
358399

359400
MethodHandle defaultTarget = callSite.getDefaultTarget();
@@ -370,7 +411,7 @@ public static Object selectMethod(CacheableCallSite callSite, Class<?> sender, S
370411
doWithCallSite(callSite, arguments, (cs, receiver) -> cs.put(receiver.getClass().getName(), mhw));
371412
}
372413

373-
return mhw.getCachedMethodHandle().invokeExact(arguments);
414+
return mhw.getCachedMethodHandle();
374415
}
375416

376417
private static MethodHandleWrapper fallback(CacheableCallSite callSite, Class<?> sender, String methodName, int callID, Boolean safeNavigation, Boolean thisCall, Boolean spreadCall, Object dummyReceiver, Object[] arguments) {

src/test/groovy/indy/IndyUsageTest.groovy

Lines changed: 0 additions & 40 deletions
This file was deleted.
Lines changed: 105 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,105 @@
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 org.codehaus.groovy.vmplugin.v8
20+
21+
import org.junit.jupiter.api.Test
22+
23+
import java.lang.invoke.MethodHandles
24+
import java.lang.invoke.MethodType
25+
26+
import static org.junit.jupiter.api.Assertions.assertEquals
27+
28+
final class IndyInterfaceDeprecatedTest {
29+
30+
private String foo() {
31+
return "foo-result"
32+
}
33+
34+
@Test
35+
void testSelectMethodDeprecatedInstanceFoo() {
36+
// Prepare call site type: (IndyInterfaceDeprecatedTest) -> Object
37+
MethodHandles.Lookup lookup = MethodHandles.lookup()
38+
MethodType type = MethodType.methodType(Object, IndyInterfaceDeprecatedTest)
39+
CacheableCallSite callSite = new CacheableCallSite(type, lookup)
40+
41+
// Provide non-null default/fallback targets (needed for guards in Selector)
42+
def dummyTarget = MethodHandles.dropArguments(
43+
MethodHandles.constant(Object, null), 0, type.parameterArray()
44+
)
45+
callSite.defaultTarget = dummyTarget
46+
callSite.fallbackTarget = dummyTarget
47+
48+
// Prepare invocation arguments
49+
def receiver = new IndyInterfaceDeprecatedTest()
50+
Object[] args = [receiver] as Object[]
51+
52+
// Call deprecated selectMethod with the requested flags
53+
int callID = IndyInterface.CallType.METHOD.getOrderNumber() // expected to be 0
54+
Object result = IndyInterface.selectMethod(
55+
callSite,
56+
IndyInterfaceDeprecatedTest, // sender
57+
'foo', // methodName
58+
callID,
59+
Boolean.FALSE, // safeNavigation
60+
Boolean.TRUE, // thisCall (instance method)
61+
Boolean.FALSE, // spreadCall
62+
1, // dummyReceiver (marker only)
63+
args
64+
)
65+
66+
// Verify the local method foo was actually called
67+
assertEquals(receiver.foo(), result)
68+
}
69+
70+
@Test
71+
void testFromCacheDeprecatedInstanceFoo() {
72+
// Prepare call site type: (IndyInterfaceDeprecatedTest) -> Object
73+
MethodHandles.Lookup lookup = MethodHandles.lookup()
74+
MethodType type = MethodType.methodType(Object, IndyInterfaceDeprecatedTest)
75+
CacheableCallSite callSite = new CacheableCallSite(type, lookup)
76+
77+
// Provide non-null default/fallback targets (needed for guards in Selector)
78+
def dummyTarget = MethodHandles.dropArguments(
79+
MethodHandles.constant(Object, null), 0, type.parameterArray()
80+
)
81+
callSite.defaultTarget = dummyTarget
82+
callSite.fallbackTarget = dummyTarget
83+
84+
// Prepare invocation arguments
85+
def receiver = new IndyInterfaceDeprecatedTest()
86+
Object[] args = [receiver] as Object[]
87+
88+
// Call deprecated fromCache with the requested flags
89+
int callID = IndyInterface.CallType.METHOD.getOrderNumber() // expected to be 0
90+
Object result = IndyInterface.fromCache(
91+
callSite,
92+
IndyInterfaceDeprecatedTest, // sender
93+
'foo', // methodName
94+
callID,
95+
Boolean.FALSE, // safeNavigation
96+
Boolean.TRUE, // thisCall (instance method)
97+
Boolean.FALSE, // spreadCall
98+
1, // dummyReceiver (marker only)
99+
args
100+
)
101+
102+
// Verify the local method foo was actually called
103+
assertEquals(receiver.foo(), result)
104+
}
105+
}

0 commit comments

Comments
 (0)