Skip to content

Commit 92adc83

Browse files
gilles-duboscqgilles-duboscq
committed
Avoid resolution under bytecode node lock during quickening
1 parent 7456cd5 commit 92adc83

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;
@@ -1754,25 +1754,26 @@ private StaticObject newReferenceArray(Klass componentType, int length) {
17541754
private BaseQuickNode getBaseQuickNode(int curBCI, int top, int statementIndex, BaseQuickNode quickNode) {
17551755
// block while class redefinition is ongoing
17561756
getMethod().getContext().getClassRedefinition().check();
1757-
BaseQuickNode result = quickNode;
1758-
result = atomic(() -> {
1759-
// re-check if node was already replaced by another thread
1760-
if (quickNode != nodes[readCPI(curBCI)]) {
1757+
// re-check if node was already replaced by another thread
1758+
if (quickNode != nodes[readCPI(curBCI)]) {
1759+
// another thread beat us
1760+
return nodes[readCPI(curBCI)];
1761+
}
1762+
BytecodeStream original = new BytecodeStream(getMethodVersion().getCodeAttribute().getOriginalCode());
1763+
char originalCpi = original.readCPI(curBCI);
1764+
int originalOpcode = original.currentBC(curBCI);
1765+
ResolvedInvoke resolvedInvoke = reResolvedInvoke(originalOpcode, originalCpi);
1766+
return atomic(() -> {
1767+
char cpi = readCPI(curBCI);
1768+
if (quickNode != nodes[cpi]) {
17611769
// another thread beat us
1762-
return nodes[readCPI(curBCI)];
1770+
return nodes[cpi];
17631771
} else {
1764-
// other threads might still have beat us but if
1765-
// so, the resolution failed and so will we below
1766-
BytecodeStream original = new BytecodeStream(getMethodVersion().getCodeAttribute().getOriginalCode());
1767-
char cpi = original.readCPI(curBCI);
1768-
int nodeOpcode = original.currentBC(curBCI);
1769-
Method resolutionSeed = resolveMethodNoCache(nodeOpcode, cpi);
1770-
BaseQuickNode toInsert = insert(dispatchQuickened(top, curBCI, cpi, nodeOpcode, statementIndex, resolutionSeed, getMethod().getContext().getEspressoEnv().bytecodeLevelInlining));
1771-
nodes[readCPI(curBCI)] = toInsert;
1772-
return toInsert;
1772+
BaseQuickNode newNode = insert(dispatchQuickened(top, curBCI, originalOpcode, statementIndex, resolvedInvoke, getMethod().getContext().getEspressoEnv().bytecodeLevelInlining));
1773+
nodes[cpi] = newNode;
1774+
return newNode;
17731775
}
17741776
});
1775-
return result;
17761777
}
17771778

17781779
private Object getReturnValueAsObject(VirtualFrame frame, int top) {
@@ -2192,20 +2193,52 @@ private BaseQuickNode injectQuick(int curBCI, BaseQuickNode quick, int opcode) {
21922193
return quick;
21932194
}
21942195

2195-
private BaseQuickNode tryPatchQuick(int curBCI, Supplier<BaseQuickNode> newQuickNode) {
2196+
@FunctionalInterface
2197+
private interface QuickNodeFactory<T> {
2198+
BaseQuickNode get(T t);
2199+
}
2200+
2201+
@FunctionalInterface
2202+
private interface QuickNodeResolver<T> {
2203+
T get(char cpi);
2204+
}
2205+
2206+
private <T> BaseQuickNode tryPatchQuick(int curBCI, QuickNodeResolver<T> resolver, QuickNodeFactory<T> newQuickNode) {
2207+
Object found = atomic(() -> {
2208+
if (bs.currentVolatileBC(curBCI) == QUICK) {
2209+
return nodes[readCPI(curBCI)];
2210+
} else {
2211+
return readCPI(curBCI);
2212+
}
2213+
});
2214+
if (found instanceof BaseQuickNode) {
2215+
return (BaseQuickNode) found;
2216+
}
2217+
char cpi = (char) found;
2218+
// Perform resolution outside the lock: it can call arbitrary guest code.
2219+
T resolved = resolver.get(cpi);
21962220
return atomic(() -> {
21972221
if (bs.currentVolatileBC(curBCI) == QUICK) {
21982222
return nodes[readCPI(curBCI)];
21992223
} else {
2200-
return injectQuick(curBCI, newQuickNode.get(), QUICK);
2224+
return injectQuick(curBCI, newQuickNode.get(resolved), QUICK);
22012225
}
22022226
});
22032227
}
22042228

2229+
@FunctionalInterface
2230+
private interface QuickNodeSupplier {
2231+
BaseQuickNode get();
2232+
}
2233+
2234+
private BaseQuickNode tryPatchQuick(int curBCI, QuickNodeSupplier newQuickNode) {
2235+
return tryPatchQuick(curBCI, cpi -> null, unused -> newQuickNode.get());
2236+
}
2237+
22052238
private int quickenCheckCast(VirtualFrame frame, int top, int curBCI, int opcode) {
22062239
CompilerAsserts.neverPartOfCompilation();
22072240
assert opcode == CHECKCAST;
2208-
BaseQuickNode quick = tryPatchQuick(curBCI, () -> new CheckCastQuickNode(resolveType(CHECKCAST, readCPI(curBCI)), top, curBCI));
2241+
BaseQuickNode quick = tryPatchQuick(curBCI, cpi -> resolveType(CHECKCAST, cpi), k -> new CheckCastQuickNode(k, top, curBCI));
22092242
quick.execute(frame, false);
22102243
assert Bytecodes.stackEffectOf(opcode) == 0;
22112244
return 0; // Bytecodes.stackEffectOf(opcode);
@@ -2214,7 +2247,7 @@ private int quickenCheckCast(VirtualFrame frame, int top, int curBCI, int opcode
22142247
private int quickenInstanceOf(VirtualFrame frame, int top, int curBCI, int opcode) {
22152248
CompilerAsserts.neverPartOfCompilation();
22162249
assert opcode == INSTANCEOF;
2217-
BaseQuickNode quick = tryPatchQuick(curBCI, () -> new InstanceOfQuickNode(resolveType(INSTANCEOF, readCPI(curBCI)), top, curBCI));
2250+
BaseQuickNode quick = tryPatchQuick(curBCI, cpi -> resolveType(INSTANCEOF, cpi), k -> new InstanceOfQuickNode(k, top, curBCI));
22182251
quick.execute(frame, false);
22192252
assert Bytecodes.stackEffectOf(opcode) == 0;
22202253
return 0; // Bytecodes.stackEffectOf(opcode);
@@ -2240,24 +2273,19 @@ private InvokeQuickNode quickenInvoke(int top, int curBCI, int opcode, int state
22402273
QUICKENED_INVOKES.inc();
22412274
CompilerDirectives.transferToInterpreterAndInvalidate();
22422275
assert Bytecodes.isInvoke(opcode);
2243-
InvokeQuickNode quick = (InvokeQuickNode) tryPatchQuick(curBCI, () -> {
2244-
// During resolution of the symbolic reference to the method, any of the exceptions
2245-
// pertaining to method resolution (&sect;5.4.3.3) can be thrown.
2246-
char cpi = readCPI(curBCI);
2247-
Method resolutionSeed = resolveMethod(opcode, cpi);
2248-
return dispatchQuickened(top, curBCI, cpi, opcode, statementIndex, resolutionSeed, getMethod().getContext().getEspressoEnv().bytecodeLevelInlining);
2249-
});
2276+
InvokeQuickNode quick = (InvokeQuickNode) tryPatchQuick(curBCI, cpi -> getResolvedInvoke(opcode, cpi),
2277+
resolvedInvoke -> dispatchQuickened(top, curBCI, opcode, statementIndex, resolvedInvoke, getMethod().getContext().getEspressoEnv().bytecodeLevelInlining));
22502278
return quick;
22512279
}
22522280

22532281
/**
22542282
* Revert speculative quickening e.g. revert inlined fields accessors to a normal invoke.
22552283
* INVOKEVIRTUAL -> QUICK (InlinedGetter/SetterNode) -> QUICK (InvokeVirtualNode)
22562284
*/
2257-
public int reQuickenInvoke(VirtualFrame frame, int top, int opcode, int curBCI, int statementIndex, Method resolutionSeed) {
2285+
public int reQuickenInvoke(VirtualFrame frame, int top, int opcode, int curBCI, int statementIndex) {
22582286
CompilerAsserts.neverPartOfCompilation();
22592287
assert Bytecodes.isInvoke(opcode);
2260-
BaseQuickNode invoke = generifyInlinedMethodNode(top, opcode, curBCI, statementIndex, resolutionSeed);
2288+
BaseQuickNode invoke = generifyInlinedMethodNode(top, opcode, curBCI, statementIndex);
22612289
// Perform the call outside of the lock.
22622290
return invoke.execute(frame, false);
22632291
}
@@ -2292,8 +2320,9 @@ private BaseQuickNode replaceQuickAt(int opcode, int curBCI, BaseQuickNode old,
22922320
* Reverts Bytecode-level method inlining at the current bci, in case instrumentation starts
22932321
* happening on this node.
22942322
*/
2295-
public BaseQuickNode generifyInlinedMethodNode(int top, int opcode, int curBCI, int statementIndex, Method resolutionSeed) {
2323+
public BaseQuickNode generifyInlinedMethodNode(int top, int opcode, int curBCI, int statementIndex) {
22962324
CompilerAsserts.neverPartOfCompilation();
2325+
ResolvedInvoke resolvedInvoke = getResolvedInvoke(opcode, readOriginalCPI(curBCI));
22972326
return atomic(() -> {
22982327
assert bs.currentBC(curBCI) == QUICK;
22992328
char nodeIndex = readCPI(curBCI);
@@ -2303,7 +2332,7 @@ public BaseQuickNode generifyInlinedMethodNode(int top, int opcode, int curBCI,
23032332
// Might be racy, as read is not volatile, but redoing the work should be OK.
23042333
return currentQuick;
23052334
}
2306-
BaseQuickNode invoke = dispatchQuickened(top, curBCI, readOriginalCPI(curBCI), opcode, statementIndex, resolutionSeed, false);
2335+
BaseQuickNode invoke = dispatchQuickened(top, curBCI, opcode, statementIndex, resolvedInvoke, false);
23072336
nodes[nodeIndex] = currentQuick.replace(invoke);
23082337
return invoke;
23092338
});
@@ -2409,12 +2438,8 @@ private int quickenArrayStore(final VirtualFrame frame, int top, int curBCI, int
24092438

24102439
// endregion quickenForeign
24112440

2412-
private InvokeQuickNode dispatchQuickened(int top, int curBCI, char cpi, int opcode, int statementIndex, Method resolutionSeed, boolean allowBytecodeInlining) {
2413-
2414-
Klass symbolicRef = Resolution.getResolvedHolderKlass((MethodRefConstant.Indexes) getConstantPool().methodAt(cpi), getConstantPool(), getDeclaringKlass());
2415-
ResolvedCall<Klass, Method, Field> resolvedCall = EspressoLinkResolver.resolveCallSite(getContext(),
2416-
getDeclaringKlass(), resolutionSeed, SiteTypes.callSiteFromOpCode(opcode), symbolicRef);
2417-
2441+
private InvokeQuickNode dispatchQuickened(int top, int curBCI, int opcode, int statementIndex, ResolvedInvoke resolvedInvoke, boolean allowBytecodeInlining) {
2442+
ResolvedCall<Klass, Method, Field> resolvedCall = resolvedInvoke.resolvedCall();
24182443
Method resolved = resolvedCall.getResolvedMethod();
24192444
CallKind callKind = resolvedCall.getCallKind();
24202445

@@ -2430,13 +2455,7 @@ private InvokeQuickNode dispatchQuickened(int top, int curBCI, char cpi, int opc
24302455
}
24312456

24322457
if (resolved.isPolySignatureIntrinsic()) {
2433-
MethodHandleInvoker invoker = null;
2434-
// There might be an invoker if it's an InvokeGeneric
2435-
if (getConstantPool().resolvedMethodRefAt(getDeclaringKlass(), cpi) instanceof ResolvedWithInvokerClassMethodRefConstant withInvoker) {
2436-
invoker = withInvoker.invoker();
2437-
assert invoker == null || ((opcode == INVOKEVIRTUAL || opcode == INVOKESPECIAL) && resolved.isInvokeIntrinsic());
2438-
}
2439-
return new InvokeHandleNode(resolved, invoker, top, curBCI);
2458+
return new InvokeHandleNode(resolved, resolvedInvoke.invoker(), top, curBCI);
24402459
} else {
24412460
// @formatter:off
24422461
return switch (callKind) {
@@ -2466,39 +2485,10 @@ private RuntimeException throwBoundary(ObjectKlass exceptionKlass, String messag
24662485

24672486
private int quickenInvokeDynamic(final VirtualFrame frame, int top, int curBCI, int opcode) {
24682487
CompilerDirectives.transferToInterpreterAndInvalidate();
2469-
assert (Bytecodes.INVOKEDYNAMIC == opcode);
2470-
RuntimeConstantPool pool = getConstantPool();
2471-
BaseQuickNode quick = null;
2472-
int indyIndex = -1;
2473-
Lock lock = getLock();
2474-
try {
2475-
lock.lock();
2476-
if (bs.currentVolatileBC(curBCI) == QUICK) {
2477-
// Check if someone did the job for us. Defer the call until we are out of the lock.
2478-
quick = nodes[readCPI(curBCI)];
2479-
} else {
2480-
// fetch indy under lock.
2481-
indyIndex = readCPI(curBCI);
2482-
}
2483-
} finally {
2484-
lock.unlock();
2485-
}
2486-
if (quick != null) {
2487-
// Do invocation outside of the lock.
2488-
return quick.execute(frame, false) - Bytecodes.stackEffectOf(opcode);
2489-
}
2490-
// Resolution should happen outside of the bytecode patching lock.
2491-
CallSiteLink link = pool.linkInvokeDynamic(getMethod().getDeclaringKlass(), indyIndex, curBCI, getMethod());
2492-
2493-
// re-lock to check if someone did the job for us, since this was a heavy operation.
2494-
quick = atomic(() -> {
2495-
if (bs.currentVolatileBC(curBCI) == QUICK) {
2496-
// someone beat us to it, just trust him.
2497-
return nodes[readCPI(curBCI)];
2498-
} else {
2499-
return injectQuick(curBCI, new InvokeDynamicCallSiteNode(link.getMemberName(), link.getUnboxedAppendix(), link.getParsedSignature(), getMethod().getMeta(), top, curBCI), QUICK);
2500-
}
2501-
});
2488+
assert opcode == Bytecodes.INVOKEDYNAMIC;
2489+
BaseQuickNode quick = tryPatchQuick(curBCI,
2490+
cpi -> getConstantPool().linkInvokeDynamic(getMethod().getDeclaringKlass(), cpi, curBCI, getMethod()),
2491+
link -> new InvokeDynamicCallSiteNode(link.getMemberName(), link.getUnboxedAppendix(), link.getParsedSignature(), getMethod().getMeta(), top, curBCI));
25022492
return quick.execute(frame, false) - Bytecodes.stackEffectOf(opcode);
25032493
}
25042494

@@ -2512,17 +2502,6 @@ public Klass resolveType(int opcode, char cpi) {
25122502
return getConstantPool().resolvedKlassAt(getDeclaringKlass(), cpi);
25132503
}
25142504

2515-
public Method resolveMethod(int opcode, char cpi) {
2516-
assert Bytecodes.isInvoke(opcode);
2517-
return getConstantPool().resolvedMethodAt(getDeclaringKlass(), cpi);
2518-
}
2519-
2520-
private Method resolveMethodNoCache(int opcode, char cpi) {
2521-
CompilerAsserts.neverPartOfCompilation();
2522-
assert Bytecodes.isInvoke(opcode);
2523-
return getConstantPool().resolvedMethodAtNoCache(getDeclaringKlass(), cpi);
2524-
}
2525-
25262505
private Field resolveField(int opcode, char cpi) {
25272506
assert opcode == GETFIELD || opcode == GETSTATIC || opcode == PUTFIELD || opcode == PUTSTATIC;
25282507
Field field = getConstantPool().resolvedFieldAt(getMethod().getDeclaringKlass(), cpi);
@@ -2534,6 +2513,34 @@ private Field resolveField(int opcode, char cpi) {
25342513
return field;
25352514
}
25362515

2516+
private record ResolvedInvoke(ResolvedCall<Klass, Method, Field> resolvedCall, MethodHandleInvoker invoker) {
2517+
}
2518+
2519+
private ResolvedInvoke reResolvedInvoke(int opcode, char cpi) {
2520+
getConstantPool().resolveMethodAndUpdate(getDeclaringKlass(), cpi);
2521+
return getResolvedInvoke(opcode, cpi);
2522+
}
2523+
2524+
private ResolvedInvoke getResolvedInvoke(int opcode, char cpi) {
2525+
assert !lockIsHeld();
2526+
// During resolution of the symbolic reference to the method, any of the exceptions
2527+
// pertaining to method resolution (&sect;5.4.3.3) can be thrown.
2528+
MethodRefConstant methodRefConstant = getConstantPool().resolvedMethodRefAt(getDeclaringKlass(), cpi);
2529+
Method resolutionSeed = (Method) ((Resolvable.ResolvedConstant) methodRefConstant).value();
2530+
2531+
Klass symbolicRef = Resolution.getResolvedHolderKlass((MethodRefConstant.Indexes) getConstantPool().methodAt(cpi), getConstantPool(), getDeclaringKlass());
2532+
CallSiteType callSiteType = SiteTypes.callSiteFromOpCode(opcode);
2533+
ResolvedCall<Klass, Method, Field> resolvedCall = EspressoLinkResolver.resolveCallSite(getContext(), getDeclaringKlass(), resolutionSeed, callSiteType, symbolicRef);
2534+
MethodHandleInvoker invoker = null;
2535+
// There might be an invoker if it's an InvokeGeneric
2536+
if (methodRefConstant instanceof ResolvedWithInvokerClassMethodRefConstant withInvoker) {
2537+
invoker = withInvoker.invoker();
2538+
assert invoker == null || ((opcode == INVOKEVIRTUAL || opcode == INVOKESPECIAL) && resolvedCall.getResolvedMethod().isInvokeIntrinsic());
2539+
}
2540+
2541+
return new ResolvedInvoke(resolvedCall, invoker);
2542+
}
2543+
25372544
// endregion Class/Method/Field resolution
25382545

25392546
// 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)