Skip to content

Commit ff5ecc8

Browse files
gilles-duboscqgilles-duboscq
committed
[GR-60641] Avoid resolution under bytecode node lock during quickening.
PullRequest: graal/19659
2 parents 21b0782 + 92adc83 commit ff5ecc8

File tree

5 files changed

+104
-94
lines changed

5 files changed

+104
-94
lines changed

‎espresso/src/com.oracle.truffle.espresso/src/com/oracle/truffle/espresso/constantpool/RuntimeConstantPool.java‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -199,10 +199,13 @@ public MethodRefConstant resolvedMethodRefAt(ObjectKlass accessingKlass, int ind
199199
return (MethodRefConstant) resolved;
200200
}
201201

202-
public Method resolvedMethodAtNoCache(ObjectKlass accessingKlass, int index) {
202+
public Method resolveMethodAndUpdate(ObjectKlass accessingKlass, int index) {
203203
CompilerAsserts.neverPartOfCompilation();
204204
Resolvable.ResolvedConstant resolved = resolvedAtNoCache(accessingKlass, index, "method");
205-
return (Method) resolved.value();
205+
synchronized (this) {
206+
resolvedConstants[index] = resolved;
207+
}
208+
return ((Method) resolved.value());
206209
}
207210

208211
public StaticObject resolvedMethodHandleAt(ObjectKlass accessingKlass, int index) {

‎espresso/src/com.oracle.truffle.espresso/src/com/oracle/truffle/espresso/nodes/BytecodeNode.java‎

Lines changed: 96 additions & 89 deletions
Original file line numberDiff line numberDiff line change
@@ -275,7 +275,6 @@
275275
import java.util.Set;
276276
import java.util.concurrent.locks.Lock;
277277
import java.util.concurrent.locks.ReentrantLock;
278-
import java.util.function.Supplier;
279278

280279
import com.oracle.truffle.api.Assumption;
281280
import com.oracle.truffle.api.CompilerAsserts;
@@ -324,12 +323,12 @@
324323
import com.oracle.truffle.espresso.classfile.constantpool.MethodRefConstant;
325324
import com.oracle.truffle.espresso.classfile.constantpool.MethodTypeConstant;
326325
import com.oracle.truffle.espresso.classfile.constantpool.PoolConstant;
326+
import com.oracle.truffle.espresso.classfile.constantpool.Resolvable;
327327
import com.oracle.truffle.espresso.classfile.constantpool.StringConstant;
328328
import com.oracle.truffle.espresso.classfile.descriptors.Signatures;
329329
import com.oracle.truffle.espresso.classfile.descriptors.Symbol;
330330
import com.oracle.truffle.espresso.classfile.descriptors.Symbol.Type;
331331
import com.oracle.truffle.espresso.classfile.perf.DebugCounter;
332-
import com.oracle.truffle.espresso.constantpool.CallSiteLink;
333332
import com.oracle.truffle.espresso.constantpool.Resolution;
334333
import com.oracle.truffle.espresso.constantpool.ResolvedDynamicConstant;
335334
import com.oracle.truffle.espresso.constantpool.ResolvedWithInvokerClassMethodRefConstant;
@@ -383,6 +382,7 @@
383382
import com.oracle.truffle.espresso.runtime.GuestAllocator;
384383
import com.oracle.truffle.espresso.runtime.staticobject.StaticObject;
385384
import com.oracle.truffle.espresso.shared.resolver.CallKind;
385+
import com.oracle.truffle.espresso.shared.resolver.CallSiteType;
386386
import com.oracle.truffle.espresso.shared.resolver.FieldAccessType;
387387
import com.oracle.truffle.espresso.shared.resolver.ResolvedCall;
388388
import com.oracle.truffle.espresso.substitutions.Target_java_lang_invoke_MethodHandleNatives.SiteTypes;
@@ -1752,25 +1752,26 @@ private StaticObject newReferenceArray(Klass componentType, int length) {
17521752
private BaseQuickNode getBaseQuickNode(int curBCI, int top, int statementIndex, BaseQuickNode quickNode) {
17531753
// block while class redefinition is ongoing
17541754
getMethod().getContext().getClassRedefinition().check();
1755-
BaseQuickNode result = quickNode;
1756-
result = atomic(() -> {
1757-
// re-check if node was already replaced by another thread
1758-
if (quickNode != nodes[readCPI(curBCI)]) {
1755+
// re-check if node was already replaced by another thread
1756+
if (quickNode != nodes[readCPI(curBCI)]) {
1757+
// another thread beat us
1758+
return nodes[readCPI(curBCI)];
1759+
}
1760+
BytecodeStream original = new BytecodeStream(getMethodVersion().getCodeAttribute().getOriginalCode());
1761+
char originalCpi = original.readCPI(curBCI);
1762+
int originalOpcode = original.currentBC(curBCI);
1763+
ResolvedInvoke resolvedInvoke = reResolvedInvoke(originalOpcode, originalCpi);
1764+
return atomic(() -> {
1765+
char cpi = readCPI(curBCI);
1766+
if (quickNode != nodes[cpi]) {
17591767
// another thread beat us
1760-
return nodes[readCPI(curBCI)];
1768+
return nodes[cpi];
17611769
} else {
1762-
// other threads might still have beat us but if
1763-
// so, the resolution failed and so will we below
1764-
BytecodeStream original = new BytecodeStream(getMethodVersion().getCodeAttribute().getOriginalCode());
1765-
char cpi = original.readCPI(curBCI);
1766-
int nodeOpcode = original.currentBC(curBCI);
1767-
Method resolutionSeed = resolveMethodNoCache(nodeOpcode, cpi);
1768-
BaseQuickNode toInsert = insert(dispatchQuickened(top, curBCI, cpi, nodeOpcode, statementIndex, resolutionSeed, getMethod().getContext().getEspressoEnv().bytecodeLevelInlining));
1769-
nodes[readCPI(curBCI)] = toInsert;
1770-
return toInsert;
1770+
BaseQuickNode newNode = insert(dispatchQuickened(top, curBCI, originalOpcode, statementIndex, resolvedInvoke, getMethod().getContext().getEspressoEnv().bytecodeLevelInlining));
1771+
nodes[cpi] = newNode;
1772+
return newNode;
17711773
}
17721774
});
1773-
return result;
17741775
}
17751776

17761777
private Object getReturnValueAsObject(VirtualFrame frame, int top) {
@@ -2179,20 +2180,52 @@ private BaseQuickNode injectQuick(int curBCI, BaseQuickNode quick, int opcode) {
21792180
return quick;
21802181
}
21812182

2182-
private BaseQuickNode tryPatchQuick(int curBCI, Supplier<BaseQuickNode> newQuickNode) {
2183+
@FunctionalInterface
2184+
private interface QuickNodeFactory<T> {
2185+
BaseQuickNode get(T t);
2186+
}
2187+
2188+
@FunctionalInterface
2189+
private interface QuickNodeResolver<T> {
2190+
T get(char cpi);
2191+
}
2192+
2193+
private <T> BaseQuickNode tryPatchQuick(int curBCI, QuickNodeResolver<T> resolver, QuickNodeFactory<T> newQuickNode) {
2194+
Object found = atomic(() -> {
2195+
if (bs.currentVolatileBC(curBCI) == QUICK) {
2196+
return nodes[readCPI(curBCI)];
2197+
} else {
2198+
return readCPI(curBCI);
2199+
}
2200+
});
2201+
if (found instanceof BaseQuickNode) {
2202+
return (BaseQuickNode) found;
2203+
}
2204+
char cpi = (char) found;
2205+
// Perform resolution outside the lock: it can call arbitrary guest code.
2206+
T resolved = resolver.get(cpi);
21832207
return atomic(() -> {
21842208
if (bs.currentVolatileBC(curBCI) == QUICK) {
21852209
return nodes[readCPI(curBCI)];
21862210
} else {
2187-
return injectQuick(curBCI, newQuickNode.get(), QUICK);
2211+
return injectQuick(curBCI, newQuickNode.get(resolved), QUICK);
21882212
}
21892213
});
21902214
}
21912215

2216+
@FunctionalInterface
2217+
private interface QuickNodeSupplier {
2218+
BaseQuickNode get();
2219+
}
2220+
2221+
private BaseQuickNode tryPatchQuick(int curBCI, QuickNodeSupplier newQuickNode) {
2222+
return tryPatchQuick(curBCI, cpi -> null, unused -> newQuickNode.get());
2223+
}
2224+
21922225
private int quickenCheckCast(VirtualFrame frame, int top, int curBCI, int opcode) {
21932226
CompilerAsserts.neverPartOfCompilation();
21942227
assert opcode == CHECKCAST;
2195-
BaseQuickNode quick = tryPatchQuick(curBCI, () -> new CheckCastQuickNode(resolveType(CHECKCAST, readCPI(curBCI)), top, curBCI));
2228+
BaseQuickNode quick = tryPatchQuick(curBCI, cpi -> resolveType(CHECKCAST, cpi), k -> new CheckCastQuickNode(k, top, curBCI));
21962229
quick.execute(frame, false);
21972230
assert Bytecodes.stackEffectOf(opcode) == 0;
21982231
return 0; // Bytecodes.stackEffectOf(opcode);
@@ -2201,7 +2234,7 @@ private int quickenCheckCast(VirtualFrame frame, int top, int curBCI, int opcode
22012234
private int quickenInstanceOf(VirtualFrame frame, int top, int curBCI, int opcode) {
22022235
CompilerAsserts.neverPartOfCompilation();
22032236
assert opcode == INSTANCEOF;
2204-
BaseQuickNode quick = tryPatchQuick(curBCI, () -> new InstanceOfQuickNode(resolveType(INSTANCEOF, readCPI(curBCI)), top, curBCI));
2237+
BaseQuickNode quick = tryPatchQuick(curBCI, cpi -> resolveType(INSTANCEOF, cpi), k -> new InstanceOfQuickNode(k, top, curBCI));
22052238
quick.execute(frame, false);
22062239
assert Bytecodes.stackEffectOf(opcode) == 0;
22072240
return 0; // Bytecodes.stackEffectOf(opcode);
@@ -2227,24 +2260,19 @@ private InvokeQuickNode quickenInvoke(int top, int curBCI, int opcode, int state
22272260
QUICKENED_INVOKES.inc();
22282261
CompilerDirectives.transferToInterpreterAndInvalidate();
22292262
assert Bytecodes.isInvoke(opcode);
2230-
InvokeQuickNode quick = (InvokeQuickNode) tryPatchQuick(curBCI, () -> {
2231-
// During resolution of the symbolic reference to the method, any of the exceptions
2232-
// pertaining to method resolution (&sect;5.4.3.3) can be thrown.
2233-
char cpi = readCPI(curBCI);
2234-
Method resolutionSeed = resolveMethod(opcode, cpi);
2235-
return dispatchQuickened(top, curBCI, cpi, opcode, statementIndex, resolutionSeed, getMethod().getContext().getEspressoEnv().bytecodeLevelInlining);
2236-
});
2263+
InvokeQuickNode quick = (InvokeQuickNode) tryPatchQuick(curBCI, cpi -> getResolvedInvoke(opcode, cpi),
2264+
resolvedInvoke -> dispatchQuickened(top, curBCI, opcode, statementIndex, resolvedInvoke, getMethod().getContext().getEspressoEnv().bytecodeLevelInlining));
22372265
return quick;
22382266
}
22392267

22402268
/**
22412269
* Revert speculative quickening e.g. revert inlined fields accessors to a normal invoke.
22422270
* INVOKEVIRTUAL -> QUICK (InlinedGetter/SetterNode) -> QUICK (InvokeVirtualNode)
22432271
*/
2244-
public int reQuickenInvoke(VirtualFrame frame, int top, int opcode, int curBCI, int statementIndex, Method resolutionSeed) {
2272+
public int reQuickenInvoke(VirtualFrame frame, int top, int opcode, int curBCI, int statementIndex) {
22452273
CompilerAsserts.neverPartOfCompilation();
22462274
assert Bytecodes.isInvoke(opcode);
2247-
BaseQuickNode invoke = generifyInlinedMethodNode(top, opcode, curBCI, statementIndex, resolutionSeed);
2275+
BaseQuickNode invoke = generifyInlinedMethodNode(top, opcode, curBCI, statementIndex);
22482276
// Perform the call outside of the lock.
22492277
return invoke.execute(frame, false);
22502278
}
@@ -2279,8 +2307,9 @@ private BaseQuickNode replaceQuickAt(int opcode, int curBCI, BaseQuickNode old,
22792307
* Reverts Bytecode-level method inlining at the current bci, in case instrumentation starts
22802308
* happening on this node.
22812309
*/
2282-
public BaseQuickNode generifyInlinedMethodNode(int top, int opcode, int curBCI, int statementIndex, Method resolutionSeed) {
2310+
public BaseQuickNode generifyInlinedMethodNode(int top, int opcode, int curBCI, int statementIndex) {
22832311
CompilerAsserts.neverPartOfCompilation();
2312+
ResolvedInvoke resolvedInvoke = getResolvedInvoke(opcode, readOriginalCPI(curBCI));
22842313
return atomic(() -> {
22852314
assert bs.currentBC(curBCI) == QUICK;
22862315
char nodeIndex = readCPI(curBCI);
@@ -2290,7 +2319,7 @@ public BaseQuickNode generifyInlinedMethodNode(int top, int opcode, int curBCI,
22902319
// Might be racy, as read is not volatile, but redoing the work should be OK.
22912320
return currentQuick;
22922321
}
2293-
BaseQuickNode invoke = dispatchQuickened(top, curBCI, readOriginalCPI(curBCI), opcode, statementIndex, resolutionSeed, false);
2322+
BaseQuickNode invoke = dispatchQuickened(top, curBCI, opcode, statementIndex, resolvedInvoke, false);
22942323
nodes[nodeIndex] = currentQuick.replace(invoke);
22952324
return invoke;
22962325
});
@@ -2396,12 +2425,8 @@ private int quickenArrayStore(final VirtualFrame frame, int top, int curBCI, int
23962425

23972426
// endregion quickenForeign
23982427

2399-
private InvokeQuickNode dispatchQuickened(int top, int curBCI, char cpi, int opcode, int statementIndex, Method resolutionSeed, boolean allowBytecodeInlining) {
2400-
2401-
Klass symbolicRef = Resolution.getResolvedHolderKlass((MethodRefConstant.Indexes) getConstantPool().methodAt(cpi), getConstantPool(), getDeclaringKlass());
2402-
ResolvedCall<Klass, Method, Field> resolvedCall = EspressoLinkResolver.resolveCallSite(getContext(),
2403-
getDeclaringKlass(), resolutionSeed, SiteTypes.callSiteFromOpCode(opcode), symbolicRef);
2404-
2428+
private InvokeQuickNode dispatchQuickened(int top, int curBCI, int opcode, int statementIndex, ResolvedInvoke resolvedInvoke, boolean allowBytecodeInlining) {
2429+
ResolvedCall<Klass, Method, Field> resolvedCall = resolvedInvoke.resolvedCall();
24052430
Method resolved = resolvedCall.getResolvedMethod();
24062431
CallKind callKind = resolvedCall.getCallKind();
24072432

@@ -2417,13 +2442,7 @@ private InvokeQuickNode dispatchQuickened(int top, int curBCI, char cpi, int opc
24172442
}
24182443

24192444
if (resolved.isPolySignatureIntrinsic()) {
2420-
MethodHandleInvoker invoker = null;
2421-
// There might be an invoker if it's an InvokeGeneric
2422-
if (getConstantPool().resolvedMethodRefAt(getDeclaringKlass(), cpi) instanceof ResolvedWithInvokerClassMethodRefConstant withInvoker) {
2423-
invoker = withInvoker.invoker();
2424-
assert invoker == null || ((opcode == INVOKEVIRTUAL || opcode == INVOKESPECIAL) && resolved.isInvokeIntrinsic());
2425-
}
2426-
return new InvokeHandleNode(resolved, invoker, top, curBCI);
2445+
return new InvokeHandleNode(resolved, resolvedInvoke.invoker(), top, curBCI);
24272446
} else {
24282447
// @formatter:off
24292448
return switch (callKind) {
@@ -2453,39 +2472,10 @@ private RuntimeException throwBoundary(ObjectKlass exceptionKlass, String messag
24532472

24542473
private int quickenInvokeDynamic(final VirtualFrame frame, int top, int curBCI, int opcode) {
24552474
CompilerDirectives.transferToInterpreterAndInvalidate();
2456-
assert (Bytecodes.INVOKEDYNAMIC == opcode);
2457-
RuntimeConstantPool pool = getConstantPool();
2458-
BaseQuickNode quick = null;
2459-
int indyIndex = -1;
2460-
Lock lock = getLock();
2461-
try {
2462-
lock.lock();
2463-
if (bs.currentVolatileBC(curBCI) == QUICK) {
2464-
// Check if someone did the job for us. Defer the call until we are out of the lock.
2465-
quick = nodes[readCPI(curBCI)];
2466-
} else {
2467-
// fetch indy under lock.
2468-
indyIndex = readCPI(curBCI);
2469-
}
2470-
} finally {
2471-
lock.unlock();
2472-
}
2473-
if (quick != null) {
2474-
// Do invocation outside of the lock.
2475-
return quick.execute(frame, false) - Bytecodes.stackEffectOf(opcode);
2476-
}
2477-
// Resolution should happen outside of the bytecode patching lock.
2478-
CallSiteLink link = pool.linkInvokeDynamic(getMethod().getDeclaringKlass(), indyIndex, curBCI, getMethod());
2479-
2480-
// re-lock to check if someone did the job for us, since this was a heavy operation.
2481-
quick = atomic(() -> {
2482-
if (bs.currentVolatileBC(curBCI) == QUICK) {
2483-
// someone beat us to it, just trust him.
2484-
return nodes[readCPI(curBCI)];
2485-
} else {
2486-
return injectQuick(curBCI, new InvokeDynamicCallSiteNode(link.getMemberName(), link.getUnboxedAppendix(), link.getParsedSignature(), getMethod().getMeta(), top, curBCI), QUICK);
2487-
}
2488-
});
2475+
assert opcode == Bytecodes.INVOKEDYNAMIC;
2476+
BaseQuickNode quick = tryPatchQuick(curBCI,
2477+
cpi -> getConstantPool().linkInvokeDynamic(getMethod().getDeclaringKlass(), cpi, curBCI, getMethod()),
2478+
link -> new InvokeDynamicCallSiteNode(link.getMemberName(), link.getUnboxedAppendix(), link.getParsedSignature(), getMethod().getMeta(), top, curBCI));
24892479
return quick.execute(frame, false) - Bytecodes.stackEffectOf(opcode);
24902480
}
24912481

@@ -2499,17 +2489,6 @@ public Klass resolveType(int opcode, char cpi) {
24992489
return getConstantPool().resolvedKlassAt(getDeclaringKlass(), cpi);
25002490
}
25012491

2502-
public Method resolveMethod(int opcode, char cpi) {
2503-
assert Bytecodes.isInvoke(opcode);
2504-
return getConstantPool().resolvedMethodAt(getDeclaringKlass(), cpi);
2505-
}
2506-
2507-
private Method resolveMethodNoCache(int opcode, char cpi) {
2508-
CompilerAsserts.neverPartOfCompilation();
2509-
assert Bytecodes.isInvoke(opcode);
2510-
return getConstantPool().resolvedMethodAtNoCache(getDeclaringKlass(), cpi);
2511-
}
2512-
25132492
private Field resolveField(int opcode, char cpi) {
25142493
assert opcode == GETFIELD || opcode == GETSTATIC || opcode == PUTFIELD || opcode == PUTSTATIC;
25152494
Field field = getConstantPool().resolvedFieldAt(getMethod().getDeclaringKlass(), cpi);
@@ -2521,6 +2500,34 @@ private Field resolveField(int opcode, char cpi) {
25212500
return field;
25222501
}
25232502

2503+
private record ResolvedInvoke(ResolvedCall<Klass, Method, Field> resolvedCall, MethodHandleInvoker invoker) {
2504+
}
2505+
2506+
private ResolvedInvoke reResolvedInvoke(int opcode, char cpi) {
2507+
getConstantPool().resolveMethodAndUpdate(getDeclaringKlass(), cpi);
2508+
return getResolvedInvoke(opcode, cpi);
2509+
}
2510+
2511+
private ResolvedInvoke getResolvedInvoke(int opcode, char cpi) {
2512+
assert !lockIsHeld();
2513+
// During resolution of the symbolic reference to the method, any of the exceptions
2514+
// pertaining to method resolution (&sect;5.4.3.3) can be thrown.
2515+
MethodRefConstant methodRefConstant = getConstantPool().resolvedMethodRefAt(getDeclaringKlass(), cpi);
2516+
Method resolutionSeed = (Method) ((Resolvable.ResolvedConstant) methodRefConstant).value();
2517+
2518+
Klass symbolicRef = Resolution.getResolvedHolderKlass((MethodRefConstant.Indexes) getConstantPool().methodAt(cpi), getConstantPool(), getDeclaringKlass());
2519+
CallSiteType callSiteType = SiteTypes.callSiteFromOpCode(opcode);
2520+
ResolvedCall<Klass, Method, Field> resolvedCall = EspressoLinkResolver.resolveCallSite(getContext(), getDeclaringKlass(), resolutionSeed, callSiteType, symbolicRef);
2521+
MethodHandleInvoker invoker = null;
2522+
// There might be an invoker if it's an InvokeGeneric
2523+
if (methodRefConstant instanceof ResolvedWithInvokerClassMethodRefConstant withInvoker) {
2524+
invoker = withInvoker.invoker();
2525+
assert invoker == null || ((opcode == INVOKEVIRTUAL || opcode == INVOKESPECIAL) && resolvedCall.getResolvedMethod().isInvokeIntrinsic());
2526+
}
2527+
2528+
return new ResolvedInvoke(resolvedCall, invoker);
2529+
}
2530+
25242531
// endregion Class/Method/Field resolution
25252532

25262533
// region Instance/array allocation

‎espresso/src/com.oracle.truffle.espresso/src/com/oracle/truffle/espresso/nodes/quick/invoke/inline/GuardedConditionalInlinedMethodNode.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ public int execute(VirtualFrame frame, boolean isContinuationResume) {
6464
}
6565
} else {
6666
CompilerDirectives.transferToInterpreterAndInvalidate();
67-
return getBytecodeNode().reQuickenInvoke(frame, top, opcode, getCallerBCI(), statementIndex, method.getMethod());
67+
return getBytecodeNode().reQuickenInvoke(frame, top, opcode, getCallerBCI(), statementIndex);
6868
}
6969
}
7070

‎espresso/src/com.oracle.truffle.espresso/src/com/oracle/truffle/espresso/nodes/quick/invoke/inline/GuardedInlinedMethodNode.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ public int execute(VirtualFrame frame, boolean isContinuationResume) {
4646
return executeBody(frame);
4747
} else {
4848
CompilerDirectives.transferToInterpreterAndInvalidate();
49-
return getBytecodeNode().reQuickenInvoke(frame, top, opcode, getCallerBCI(), statementIndex, method.getMethod());
49+
return getBytecodeNode().reQuickenInvoke(frame, top, opcode, getCallerBCI(), statementIndex);
5050
}
5151
}
5252
}

‎espresso/src/com.oracle.truffle.espresso/src/com/oracle/truffle/espresso/nodes/quick/invoke/inline/InlinedMethodNode.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -189,7 +189,7 @@ private void initCheck() {
189189
}
190190

191191
public final BaseQuickNode revertToGeneric(BytecodeNode parent) {
192-
return parent.generifyInlinedMethodNode(top, opcode, getCallerBCI(), statementIndex, method.getMethod());
192+
return parent.generifyInlinedMethodNode(top, opcode, getCallerBCI(), statementIndex);
193193
}
194194

195195
public static boolean isInlineCandidate(ResolvedCall<Klass, Method, Field> resolvedCall) {

0 commit comments

Comments
 (0)