Skip to content

Commit

Permalink
[GR-60641] Avoid resolution under bytecode node lock during quickening.
Browse files Browse the repository at this point in the history 
PullRequest: graal/19659
  • Loading branch information
@gilles-duboscq
gilles-duboscq committed Dec 27, 2024
2 parents 21b0782 + 92adc83 commit ff5ecc8
Show file tree
Hide file tree
Showing 5 changed files with 104 additions and 94 deletions.
7 changes: 5 additions & 2 deletions ...le.truffle.espresso/src/com/oracle/truffle/espresso/constantpool/RuntimeConstantPool.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -199,10 +199,13 @@ public MethodRefConstant resolvedMethodRefAt(ObjectKlass accessingKlass, int ind
return (MethodRefConstant) resolved;
}

public Method resolvedMethodAtNoCache(ObjectKlass accessingKlass, int index) {
public Method resolveMethodAndUpdate(ObjectKlass accessingKlass, int index) {
CompilerAsserts.neverPartOfCompilation();
Resolvable.ResolvedConstant resolved = resolvedAtNoCache(accessingKlass, index, "method");
return (Method) resolved.value();
synchronized (this) {
resolvedConstants[index] = resolved;
}
return ((Method) resolved.value());
}

public StaticObject resolvedMethodHandleAt(ObjectKlass accessingKlass, int index) {
Expand Down
185 changes: 96 additions & 89 deletions ...o/src/com.oracle.truffle.espresso/src/com/oracle/truffle/espresso/nodes/BytecodeNode.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -275,7 +275,6 @@
import java.util.Set;
import java.util.concurrent.locks.Lock;
import java.util.concurrent.locks.ReentrantLock;
import java.util.function.Supplier;

import com.oracle.truffle.api.Assumption;
import com.oracle.truffle.api.CompilerAsserts;
Expand Down Expand Up @@ -324,12 +323,12 @@
import com.oracle.truffle.espresso.classfile.constantpool.MethodRefConstant;
import com.oracle.truffle.espresso.classfile.constantpool.MethodTypeConstant;
import com.oracle.truffle.espresso.classfile.constantpool.PoolConstant;
import com.oracle.truffle.espresso.classfile.constantpool.Resolvable;
import com.oracle.truffle.espresso.classfile.constantpool.StringConstant;
import com.oracle.truffle.espresso.classfile.descriptors.Signatures;
import com.oracle.truffle.espresso.classfile.descriptors.Symbol;
import com.oracle.truffle.espresso.classfile.descriptors.Symbol.Type;
import com.oracle.truffle.espresso.classfile.perf.DebugCounter;
import com.oracle.truffle.espresso.constantpool.CallSiteLink;
import com.oracle.truffle.espresso.constantpool.Resolution;
import com.oracle.truffle.espresso.constantpool.ResolvedDynamicConstant;
import com.oracle.truffle.espresso.constantpool.ResolvedWithInvokerClassMethodRefConstant;
Expand Down Expand Up @@ -383,6 +382,7 @@
import com.oracle.truffle.espresso.runtime.GuestAllocator;
import com.oracle.truffle.espresso.runtime.staticobject.StaticObject;
import com.oracle.truffle.espresso.shared.resolver.CallKind;
import com.oracle.truffle.espresso.shared.resolver.CallSiteType;
import com.oracle.truffle.espresso.shared.resolver.FieldAccessType;
import com.oracle.truffle.espresso.shared.resolver.ResolvedCall;
import com.oracle.truffle.espresso.substitutions.Target_java_lang_invoke_MethodHandleNatives.SiteTypes;
Expand Down Expand Up @@ -1752,25 +1752,26 @@ private StaticObject newReferenceArray(Klass componentType, int length) {
private BaseQuickNode getBaseQuickNode(int curBCI, int top, int statementIndex, BaseQuickNode quickNode) {
// block while class redefinition is ongoing
getMethod().getContext().getClassRedefinition().check();
BaseQuickNode result = quickNode;
result = atomic(() -> {
// re-check if node was already replaced by another thread
if (quickNode != nodes[readCPI(curBCI)]) {
// re-check if node was already replaced by another thread
if (quickNode != nodes[readCPI(curBCI)]) {
// another thread beat us
return nodes[readCPI(curBCI)];
}
BytecodeStream original = new BytecodeStream(getMethodVersion().getCodeAttribute().getOriginalCode());
char originalCpi = original.readCPI(curBCI);
int originalOpcode = original.currentBC(curBCI);
ResolvedInvoke resolvedInvoke = reResolvedInvoke(originalOpcode, originalCpi);
return atomic(() -> {
char cpi = readCPI(curBCI);
if (quickNode != nodes[cpi]) {
// another thread beat us
return nodes[readCPI(curBCI)];
return nodes[cpi];
} else {
// other threads might still have beat us but if
// so, the resolution failed and so will we below
BytecodeStream original = new BytecodeStream(getMethodVersion().getCodeAttribute().getOriginalCode());
char cpi = original.readCPI(curBCI);
int nodeOpcode = original.currentBC(curBCI);
Method resolutionSeed = resolveMethodNoCache(nodeOpcode, cpi);
BaseQuickNode toInsert = insert(dispatchQuickened(top, curBCI, cpi, nodeOpcode, statementIndex, resolutionSeed, getMethod().getContext().getEspressoEnv().bytecodeLevelInlining));
nodes[readCPI(curBCI)] = toInsert;
return toInsert;
BaseQuickNode newNode = insert(dispatchQuickened(top, curBCI, originalOpcode, statementIndex, resolvedInvoke, getMethod().getContext().getEspressoEnv().bytecodeLevelInlining));
nodes[cpi] = newNode;
return newNode;
}
});
return result;
}

private Object getReturnValueAsObject(VirtualFrame frame, int top) {
Expand Down Expand Up @@ -2179,20 +2180,52 @@ private BaseQuickNode injectQuick(int curBCI, BaseQuickNode quick, int opcode) {
return quick;
}

private BaseQuickNode tryPatchQuick(int curBCI, Supplier<BaseQuickNode> newQuickNode) {
@FunctionalInterface
private interface QuickNodeFactory<T> {
BaseQuickNode get(T t);
}

@FunctionalInterface
private interface QuickNodeResolver<T> {
T get(char cpi);
}

private <T> BaseQuickNode tryPatchQuick(int curBCI, QuickNodeResolver<T> resolver, QuickNodeFactory<T> newQuickNode) {
Object found = atomic(() -> {
if (bs.currentVolatileBC(curBCI) == QUICK) {
return nodes[readCPI(curBCI)];
} else {
return readCPI(curBCI);
}
});
if (found instanceof BaseQuickNode) {
return (BaseQuickNode) found;
}
char cpi = (char) found;
// Perform resolution outside the lock: it can call arbitrary guest code.
T resolved = resolver.get(cpi);
return atomic(() -> {
if (bs.currentVolatileBC(curBCI) == QUICK) {
return nodes[readCPI(curBCI)];
} else {
return injectQuick(curBCI, newQuickNode.get(), QUICK);
return injectQuick(curBCI, newQuickNode.get(resolved), QUICK);
}
});
}

@FunctionalInterface
private interface QuickNodeSupplier {
BaseQuickNode get();
}

private BaseQuickNode tryPatchQuick(int curBCI, QuickNodeSupplier newQuickNode) {
return tryPatchQuick(curBCI, cpi -> null, unused -> newQuickNode.get());
}

private int quickenCheckCast(VirtualFrame frame, int top, int curBCI, int opcode) {
CompilerAsserts.neverPartOfCompilation();
assert opcode == CHECKCAST;
BaseQuickNode quick = tryPatchQuick(curBCI, () -> new CheckCastQuickNode(resolveType(CHECKCAST, readCPI(curBCI)), top, curBCI));
BaseQuickNode quick = tryPatchQuick(curBCI, cpi -> resolveType(CHECKCAST, cpi), k -> new CheckCastQuickNode(k, top, curBCI));
quick.execute(frame, false);
assert Bytecodes.stackEffectOf(opcode) == 0;
return 0; // Bytecodes.stackEffectOf(opcode);
Expand All @@ -2201,7 +2234,7 @@ private int quickenCheckCast(VirtualFrame frame, int top, int curBCI, int opcode
private int quickenInstanceOf(VirtualFrame frame, int top, int curBCI, int opcode) {
CompilerAsserts.neverPartOfCompilation();
assert opcode == INSTANCEOF;
BaseQuickNode quick = tryPatchQuick(curBCI, () -> new InstanceOfQuickNode(resolveType(INSTANCEOF, readCPI(curBCI)), top, curBCI));
BaseQuickNode quick = tryPatchQuick(curBCI, cpi -> resolveType(INSTANCEOF, cpi), k -> new InstanceOfQuickNode(k, top, curBCI));
quick.execute(frame, false);
assert Bytecodes.stackEffectOf(opcode) == 0;
return 0; // Bytecodes.stackEffectOf(opcode);
Expand All @@ -2227,24 +2260,19 @@ private InvokeQuickNode quickenInvoke(int top, int curBCI, int opcode, int state
QUICKENED_INVOKES.inc();
CompilerDirectives.transferToInterpreterAndInvalidate();
assert Bytecodes.isInvoke(opcode);
InvokeQuickNode quick = (InvokeQuickNode) tryPatchQuick(curBCI, () -> {
// During resolution of the symbolic reference to the method, any of the exceptions
// pertaining to method resolution (&sect;5.4.3.3) can be thrown.
char cpi = readCPI(curBCI);
Method resolutionSeed = resolveMethod(opcode, cpi);
return dispatchQuickened(top, curBCI, cpi, opcode, statementIndex, resolutionSeed, getMethod().getContext().getEspressoEnv().bytecodeLevelInlining);
});
InvokeQuickNode quick = (InvokeQuickNode) tryPatchQuick(curBCI, cpi -> getResolvedInvoke(opcode, cpi),
resolvedInvoke -> dispatchQuickened(top, curBCI, opcode, statementIndex, resolvedInvoke, getMethod().getContext().getEspressoEnv().bytecodeLevelInlining));
return quick;
}

/**
* Revert speculative quickening e.g. revert inlined fields accessors to a normal invoke.
* INVOKEVIRTUAL -> QUICK (InlinedGetter/SetterNode) -> QUICK (InvokeVirtualNode)
*/
public int reQuickenInvoke(VirtualFrame frame, int top, int opcode, int curBCI, int statementIndex, Method resolutionSeed) {
public int reQuickenInvoke(VirtualFrame frame, int top, int opcode, int curBCI, int statementIndex) {
CompilerAsserts.neverPartOfCompilation();
assert Bytecodes.isInvoke(opcode);
BaseQuickNode invoke = generifyInlinedMethodNode(top, opcode, curBCI, statementIndex, resolutionSeed);
BaseQuickNode invoke = generifyInlinedMethodNode(top, opcode, curBCI, statementIndex);
// Perform the call outside of the lock.
return invoke.execute(frame, false);
}
Expand Down Expand Up @@ -2279,8 +2307,9 @@ private BaseQuickNode replaceQuickAt(int opcode, int curBCI, BaseQuickNode old,
* Reverts Bytecode-level method inlining at the current bci, in case instrumentation starts
* happening on this node.
*/
public BaseQuickNode generifyInlinedMethodNode(int top, int opcode, int curBCI, int statementIndex, Method resolutionSeed) {
public BaseQuickNode generifyInlinedMethodNode(int top, int opcode, int curBCI, int statementIndex) {
CompilerAsserts.neverPartOfCompilation();
ResolvedInvoke resolvedInvoke = getResolvedInvoke(opcode, readOriginalCPI(curBCI));
return atomic(() -> {
assert bs.currentBC(curBCI) == QUICK;
char nodeIndex = readCPI(curBCI);
Expand All @@ -2290,7 +2319,7 @@ public BaseQuickNode generifyInlinedMethodNode(int top, int opcode, int curBCI,
// Might be racy, as read is not volatile, but redoing the work should be OK.
return currentQuick;
}
BaseQuickNode invoke = dispatchQuickened(top, curBCI, readOriginalCPI(curBCI), opcode, statementIndex, resolutionSeed, false);
BaseQuickNode invoke = dispatchQuickened(top, curBCI, opcode, statementIndex, resolvedInvoke, false);
nodes[nodeIndex] = currentQuick.replace(invoke);
return invoke;
});
Expand Down Expand Up @@ -2396,12 +2425,8 @@ private int quickenArrayStore(final VirtualFrame frame, int top, int curBCI, int

// endregion quickenForeign

private InvokeQuickNode dispatchQuickened(int top, int curBCI, char cpi, int opcode, int statementIndex, Method resolutionSeed, boolean allowBytecodeInlining) {

Klass symbolicRef = Resolution.getResolvedHolderKlass((MethodRefConstant.Indexes) getConstantPool().methodAt(cpi), getConstantPool(), getDeclaringKlass());
ResolvedCall<Klass, Method, Field> resolvedCall = EspressoLinkResolver.resolveCallSite(getContext(),
getDeclaringKlass(), resolutionSeed, SiteTypes.callSiteFromOpCode(opcode), symbolicRef);

private InvokeQuickNode dispatchQuickened(int top, int curBCI, int opcode, int statementIndex, ResolvedInvoke resolvedInvoke, boolean allowBytecodeInlining) {
ResolvedCall<Klass, Method, Field> resolvedCall = resolvedInvoke.resolvedCall();
Method resolved = resolvedCall.getResolvedMethod();
CallKind callKind = resolvedCall.getCallKind();

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

if (resolved.isPolySignatureIntrinsic()) {
MethodHandleInvoker invoker = null;
// There might be an invoker if it's an InvokeGeneric
if (getConstantPool().resolvedMethodRefAt(getDeclaringKlass(), cpi) instanceof ResolvedWithInvokerClassMethodRefConstant withInvoker) {
invoker = withInvoker.invoker();
assert invoker == null || ((opcode == INVOKEVIRTUAL || opcode == INVOKESPECIAL) && resolved.isInvokeIntrinsic());
}
return new InvokeHandleNode(resolved, invoker, top, curBCI);
return new InvokeHandleNode(resolved, resolvedInvoke.invoker(), top, curBCI);
} else {
// @formatter:off
return switch (callKind) {
Expand Down Expand Up @@ -2453,39 +2472,10 @@ private RuntimeException throwBoundary(ObjectKlass exceptionKlass, String messag

private int quickenInvokeDynamic(final VirtualFrame frame, int top, int curBCI, int opcode) {
CompilerDirectives.transferToInterpreterAndInvalidate();
assert (Bytecodes.INVOKEDYNAMIC == opcode);
RuntimeConstantPool pool = getConstantPool();
BaseQuickNode quick = null;
int indyIndex = -1;
Lock lock = getLock();
try {
lock.lock();
if (bs.currentVolatileBC(curBCI) == QUICK) {
// Check if someone did the job for us. Defer the call until we are out of the lock.
quick = nodes[readCPI(curBCI)];
} else {
// fetch indy under lock.
indyIndex = readCPI(curBCI);
}
} finally {
lock.unlock();
}
if (quick != null) {
// Do invocation outside of the lock.
return quick.execute(frame, false) - Bytecodes.stackEffectOf(opcode);
}
// Resolution should happen outside of the bytecode patching lock.
CallSiteLink link = pool.linkInvokeDynamic(getMethod().getDeclaringKlass(), indyIndex, curBCI, getMethod());

// re-lock to check if someone did the job for us, since this was a heavy operation.
quick = atomic(() -> {
if (bs.currentVolatileBC(curBCI) == QUICK) {
// someone beat us to it, just trust him.
return nodes[readCPI(curBCI)];
} else {
return injectQuick(curBCI, new InvokeDynamicCallSiteNode(link.getMemberName(), link.getUnboxedAppendix(), link.getParsedSignature(), getMethod().getMeta(), top, curBCI), QUICK);
}
});
assert opcode == Bytecodes.INVOKEDYNAMIC;
BaseQuickNode quick = tryPatchQuick(curBCI,
cpi -> getConstantPool().linkInvokeDynamic(getMethod().getDeclaringKlass(), cpi, curBCI, getMethod()),
link -> new InvokeDynamicCallSiteNode(link.getMemberName(), link.getUnboxedAppendix(), link.getParsedSignature(), getMethod().getMeta(), top, curBCI));
return quick.execute(frame, false) - Bytecodes.stackEffectOf(opcode);
}

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

public Method resolveMethod(int opcode, char cpi) {
assert Bytecodes.isInvoke(opcode);
return getConstantPool().resolvedMethodAt(getDeclaringKlass(), cpi);
}

private Method resolveMethodNoCache(int opcode, char cpi) {
CompilerAsserts.neverPartOfCompilation();
assert Bytecodes.isInvoke(opcode);
return getConstantPool().resolvedMethodAtNoCache(getDeclaringKlass(), cpi);
}

private Field resolveField(int opcode, char cpi) {
assert opcode == GETFIELD || opcode == GETSTATIC || opcode == PUTFIELD || opcode == PUTSTATIC;
Field field = getConstantPool().resolvedFieldAt(getMethod().getDeclaringKlass(), cpi);
Expand All @@ -2521,6 +2500,34 @@ private Field resolveField(int opcode, char cpi) {
return field;
}

private record ResolvedInvoke(ResolvedCall<Klass, Method, Field> resolvedCall, MethodHandleInvoker invoker) {
}

private ResolvedInvoke reResolvedInvoke(int opcode, char cpi) {
getConstantPool().resolveMethodAndUpdate(getDeclaringKlass(), cpi);
return getResolvedInvoke(opcode, cpi);
}

private ResolvedInvoke getResolvedInvoke(int opcode, char cpi) {
assert !lockIsHeld();
// During resolution of the symbolic reference to the method, any of the exceptions
// pertaining to method resolution (&sect;5.4.3.3) can be thrown.
MethodRefConstant methodRefConstant = getConstantPool().resolvedMethodRefAt(getDeclaringKlass(), cpi);
Method resolutionSeed = (Method) ((Resolvable.ResolvedConstant) methodRefConstant).value();

Klass symbolicRef = Resolution.getResolvedHolderKlass((MethodRefConstant.Indexes) getConstantPool().methodAt(cpi), getConstantPool(), getDeclaringKlass());
CallSiteType callSiteType = SiteTypes.callSiteFromOpCode(opcode);
ResolvedCall<Klass, Method, Field> resolvedCall = EspressoLinkResolver.resolveCallSite(getContext(), getDeclaringKlass(), resolutionSeed, callSiteType, symbolicRef);
MethodHandleInvoker invoker = null;
// There might be an invoker if it's an InvokeGeneric
if (methodRefConstant instanceof ResolvedWithInvokerClassMethodRefConstant withInvoker) {
invoker = withInvoker.invoker();
assert invoker == null || ((opcode == INVOKEVIRTUAL || opcode == INVOKESPECIAL) && resolvedCall.getResolvedMethod().isInvokeIntrinsic());
}

return new ResolvedInvoke(resolvedCall, invoker);
}

// endregion Class/Method/Field resolution

// region Instance/array allocation
Expand Down
2 changes: 1 addition & 1 deletion ...racle/truffle/espresso/nodes/quick/invoke/inline/GuardedConditionalInlinedMethodNode.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ public int execute(VirtualFrame frame, boolean isContinuationResume) {
}
} else {
CompilerDirectives.transferToInterpreterAndInvalidate();
return getBytecodeNode().reQuickenInvoke(frame, top, opcode, getCallerBCI(), statementIndex, method.getMethod());
return getBytecodeNode().reQuickenInvoke(frame, top, opcode, getCallerBCI(), statementIndex);
}
}

Expand Down
2 changes: 1 addition & 1 deletion ...o/src/com/oracle/truffle/espresso/nodes/quick/invoke/inline/GuardedInlinedMethodNode.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ public int execute(VirtualFrame frame, boolean isContinuationResume) {
return executeBody(frame);
} else {
CompilerDirectives.transferToInterpreterAndInvalidate();
return getBytecodeNode().reQuickenInvoke(frame, top, opcode, getCallerBCI(), statementIndex, method.getMethod());
return getBytecodeNode().reQuickenInvoke(frame, top, opcode, getCallerBCI(), statementIndex);
}
}
}
2 changes: 1 addition & 1 deletion ...espresso/src/com/oracle/truffle/espresso/nodes/quick/invoke/inline/InlinedMethodNode.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,7 @@ private void initCheck() {
}

public final BaseQuickNode revertToGeneric(BytecodeNode parent) {
return parent.generifyInlinedMethodNode(top, opcode, getCallerBCI(), statementIndex, method.getMethod());
return parent.generifyInlinedMethodNode(top, opcode, getCallerBCI(), statementIndex);
}

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

0 comments on commit ff5ecc8

Please sign in to comment.