Skip to content

Commit 6e3bfdb

Browse files
peter-hoferpeter-hofer
committed
[GR-38107] Avoid GetLocalInt() for Array.newInstance and prefer GetLocalInstance over GetLocalObject.
PullRequest: graal/11793
2 parents 29f6a82 + 73712bf commit 6e3bfdb

File tree

3 files changed

+48
-41
lines changed

3 files changed

+48
-41
lines changed

‎substratevm/src/com.oracle.svm.agent/src/com/oracle/svm/agent/BreakpointInterceptor.java

Lines changed: 29 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@
3636
import static com.oracle.svm.jvmtiagentbase.Support.getClassNameOrNull;
3737
import static com.oracle.svm.jvmtiagentbase.Support.getMethodDeclaringClass;
3838
import static com.oracle.svm.jvmtiagentbase.Support.getObjectArgument;
39+
import static com.oracle.svm.jvmtiagentbase.Support.getReceiver;
3940
import static com.oracle.svm.jvmtiagentbase.Support.handleException;
4041
import static com.oracle.svm.jvmtiagentbase.Support.jniFunctions;
4142
import static com.oracle.svm.jvmtiagentbase.Support.jvmtiEnv;
@@ -60,7 +61,6 @@
6061
import java.util.concurrent.locks.ReentrantLock;
6162
import java.util.function.Supplier;
6263

63-
import com.oracle.svm.jni.nativeapi.JNINativeMethod;
6464
import org.graalvm.compiler.core.common.NumUtil;
6565
import org.graalvm.nativeimage.StackValue;
6666
import org.graalvm.nativeimage.UnmanagedMemory;
@@ -76,6 +76,7 @@
7676
import org.graalvm.nativeimage.c.type.CTypeConversion;
7777
import org.graalvm.nativeimage.c.type.CTypeConversion.CCharPointerHolder;
7878
import org.graalvm.nativeimage.c.type.WordPointer;
79+
import org.graalvm.word.WordFactory;
7980

8081
import com.oracle.svm.agent.stackaccess.InterceptedState;
8182
import com.oracle.svm.agent.tracing.core.Tracer;
@@ -86,6 +87,7 @@
8687
import com.oracle.svm.jni.nativeapi.JNIEnvironment;
8788
import com.oracle.svm.jni.nativeapi.JNIFieldId;
8889
import com.oracle.svm.jni.nativeapi.JNIMethodId;
90+
import com.oracle.svm.jni.nativeapi.JNINativeMethod;
8991
import com.oracle.svm.jni.nativeapi.JNIObjectHandle;
9092
import com.oracle.svm.jni.nativeapi.JNIValue;
9193
import com.oracle.svm.jvmtiagentbase.AgentIsolate;
@@ -99,7 +101,6 @@
99101
import com.oracle.svm.jvmtiagentbase.jvmti.JvmtiEventMode;
100102
import com.oracle.svm.jvmtiagentbase.jvmti.JvmtiFrameInfo;
101103
import com.oracle.svm.jvmtiagentbase.jvmti.JvmtiLocationFormat;
102-
import org.graalvm.word.WordFactory;
103104

104105
/**
105106
* Intercepts events of interest via breakpoints in Java code.
@@ -251,7 +252,7 @@ private static boolean getDeclaredFields(JNIEnvironment jni, Breakpoint bp, Inte
251252

252253
private static boolean handleGetFields(JNIEnvironment jni, Breakpoint bp, InterceptedState state) {
253254
JNIObjectHandle callerClass = state.getDirectCallerClass();
254-
JNIObjectHandle self = getObjectArgument(0);
255+
JNIObjectHandle self = getReceiver();
255256
traceReflectBreakpoint(jni, getClassOrSingleProxyInterface(jni, self), nullHandle(), callerClass, bp.specification.methodName, null, state.getFullStackTraceOrNull());
256257
return true;
257258
}
@@ -274,7 +275,7 @@ private static boolean getDeclaredConstructors(JNIEnvironment jni, Breakpoint bp
274275

275276
private static boolean handleGetMethods(JNIEnvironment jni, Breakpoint bp, InterceptedState state) {
276277
JNIObjectHandle callerClass = state.getDirectCallerClass();
277-
JNIObjectHandle self = getObjectArgument(0);
278+
JNIObjectHandle self = getReceiver();
278279
traceReflectBreakpoint(jni, getClassOrSingleProxyInterface(jni, self), nullHandle(), callerClass, bp.specification.methodName, null, state.getFullStackTraceOrNull());
279280
return true;
280281
}
@@ -293,7 +294,7 @@ private static boolean getPermittedSubclasses(JNIEnvironment jni, Breakpoint bp,
293294

294295
private static boolean handleGetClasses(JNIEnvironment jni, Breakpoint bp, InterceptedState state) {
295296
JNIObjectHandle callerClass = state.getDirectCallerClass();
296-
JNIObjectHandle self = getObjectArgument(0);
297+
JNIObjectHandle self = getReceiver();
297298
traceReflectBreakpoint(jni, self, nullHandle(), callerClass, bp.specification.methodName, null, state.getFullStackTraceOrNull());
298299
return true;
299300
}
@@ -308,7 +309,7 @@ private static boolean getDeclaredField(JNIEnvironment jni, Breakpoint bp, Inter
308309

309310
private static boolean handleGetField(JNIEnvironment jni, Breakpoint bp, boolean declaredOnly, InterceptedState state) {
310311
JNIObjectHandle callerClass = state.getDirectCallerClass();
311-
JNIObjectHandle self = getObjectArgument(0);
312+
JNIObjectHandle self = getReceiver();
312313
JNIObjectHandle name = getObjectArgument(1);
313314
JNIObjectHandle result = Support.callObjectMethodL(jni, self, bp.method, name);
314315
if (clearException(jni)) {
@@ -369,7 +370,7 @@ private static void traceAllocateInstance(JNIEnvironment jni, JNIObjectHandle cl
369370

370371
private static boolean objectFieldOffsetByName(JNIEnvironment jni, Breakpoint bp, InterceptedState state) {
371372
JNIObjectHandle callerClass = state.getDirectCallerClass();
372-
JNIObjectHandle self = getObjectArgument(0);
373+
JNIObjectHandle self = getReceiver();
373374
JNIObjectHandle declaring = getObjectArgument(1);
374375
JNIObjectHandle name = getObjectArgument(2);
375376
Support.callLongMethodLL(jni, self, bp.method, declaring, name);
@@ -382,7 +383,7 @@ private static boolean objectFieldOffsetByName(JNIEnvironment jni, Breakpoint bp
382383

383384
private static boolean getConstructor(JNIEnvironment jni, Breakpoint bp, InterceptedState state) {
384385
JNIObjectHandle callerClass = state.getDirectCallerClass();
385-
JNIObjectHandle self = getObjectArgument(0);
386+
JNIObjectHandle self = getReceiver();
386387
JNIObjectHandle paramTypesHandle = getObjectArgument(1);
387388
JNIObjectHandle result = Support.callObjectMethodL(jni, self, bp.method, paramTypesHandle);
388389
if (clearException(jni)) {
@@ -404,7 +405,7 @@ private static boolean getDeclaredMethod(JNIEnvironment jni, Breakpoint bp, Inte
404405

405406
private static boolean handleGetMethod(JNIEnvironment jni, Breakpoint bp, boolean declaredOnly, InterceptedState state) {
406407
JNIObjectHandle callerClass = state.getDirectCallerClass();
407-
JNIObjectHandle self = getObjectArgument(0);
408+
JNIObjectHandle self = getReceiver();
408409
JNIObjectHandle nameHandle = getObjectArgument(1);
409410
JNIObjectHandle paramTypesHandle = getObjectArgument(2);
410411
JNIObjectHandle result = Support.callObjectMethodLL(jni, self, bp.method, nameHandle, paramTypesHandle);
@@ -427,7 +428,7 @@ private static boolean handleGetMethod(JNIEnvironment jni, Breakpoint bp, boolea
427428

428429
private static boolean getEnclosingMethod(JNIEnvironment jni, Breakpoint bp, InterceptedState state) {
429430
JNIObjectHandle callerClass = state.getDirectCallerClass();
430-
JNIObjectHandle self = getObjectArgument(0);
431+
JNIObjectHandle self = getReceiver();
431432
Object result = Tracer.EXPLICIT_NULL;
432433
JNIObjectHandle enclosing = Support.callObjectMethod(jni, self, bp.method);
433434
String name;
@@ -514,7 +515,7 @@ private static boolean isClassNewInstance(JNIEnvironment jni, JNIObjectHandle de
514515
}
515516

516517
private static boolean invokeConstructor(JNIEnvironment jni, Breakpoint bp, InterceptedState state) {
517-
return handleInvokeConstructor(jni, bp, state, getObjectArgument(0));
518+
return handleInvokeConstructor(jni, bp, state, getReceiver());
518519
}
519520

520521
private static boolean unreflectConstructor(JNIEnvironment jni, Breakpoint bp, InterceptedState state) {
@@ -542,7 +543,7 @@ private static boolean handleInvokeConstructor(JNIEnvironment jni, @SuppressWarn
542543

543544
private static boolean newInstance(JNIEnvironment jni, Breakpoint bp, InterceptedState state) {
544545
JNIObjectHandle callerClass = state.getDirectCallerClass();
545-
JNIObjectHandle self = getObjectArgument(0);
546+
JNIObjectHandle self = getReceiver();
546547
JNIMethodId result = newInstanceMethodID(jni, self);
547548
traceReflectBreakpoint(jni, self, nullHandle(), callerClass, bp.specification.methodName, result.notEqual(nullHandle()), state.getFullStackTraceOrNull());
548549
return true;
@@ -564,15 +565,13 @@ private static JNIMethodId newInstanceMethodID(JNIEnvironment jni, JNIObjectHand
564565
}
565566

566567
private static boolean newArrayInstance(JNIEnvironment jni, Breakpoint bp, InterceptedState state) {
567-
JNIObjectHandle componentClass = getObjectArgument(0);
568-
CIntPointer lengthPtr = StackValue.get(CIntPointer.class);
569-
boolean lengthValid = (jvmtiFunctions().GetLocalInt().invoke(jvmtiEnv(), nullHandle(), 0, 1, lengthPtr) == JvmtiError.JVMTI_ERROR_NONE);
570-
571568
JNIValue args = StackValue.get(2, JNIValue.class);
572-
args.addressOf(0).setObject(componentClass);
573-
args.addressOf(1).setInt(lengthPtr.read());
569+
args.addressOf(0).setObject(getObjectArgument(0));
570+
args.addressOf(1).setInt(0);
571+
// We ignore the actual array length because we have observed reading it to cause serious
572+
// slowdowns in multithreaded programs because it requires full safepoint operations.
574573

575-
return newArrayInstance0(jni, bp, args, lengthValid, state);
574+
return newArrayInstance0(jni, bp, args, true, state);
576575
}
577576

578577
private static boolean newArrayInstanceMulti(JNIEnvironment jni, Breakpoint bp, InterceptedState state) {
@@ -617,7 +616,7 @@ private static boolean getResources(JNIEnvironment jni, Breakpoint bp, Intercept
617616

618617
private static boolean handleGetResources(JNIEnvironment jni, Breakpoint bp, boolean returnsEnumeration, InterceptedState state) {
619618
JNIObjectHandle callerClass = state.getDirectCallerClass();
620-
JNIObjectHandle self = getObjectArgument(0);
619+
JNIObjectHandle self = getReceiver();
621620
JNIObjectHandle name = getObjectArgument(1);
622621
boolean result;
623622
JNIObjectHandle returnValue = Support.callObjectMethodL(jni, self, bp.method, name);
@@ -657,7 +656,7 @@ private static boolean getSystemResources(JNIEnvironment jni, Breakpoint bp, Int
657656

658657
private static boolean handleGetSystemResources(JNIEnvironment jni, Breakpoint bp, boolean returnsEnumeration, InterceptedState state) {
659658
JNIObjectHandle callerClass = state.getDirectCallerClass();
660-
JNIObjectHandle name = getObjectArgument(0);
659+
JNIObjectHandle name = getReceiver();
661660
JNIObjectHandle returnValue = Support.callStaticObjectMethodL(jni, bp.clazz, bp.method, name);
662661
boolean result = returnValue.notEqual(nullHandle());
663662
if (clearException(jni)) {
@@ -853,7 +852,7 @@ private static boolean loadClass(JNIEnvironment jni, Breakpoint bp, InterceptedS
853852
observedExplicitLoadClassCallSites.put(location, Boolean.TRUE);
854853
}
855854
}
856-
JNIObjectHandle self = getObjectArgument(0);
855+
JNIObjectHandle self = getReceiver();
857856
JNIObjectHandle name = getObjectArgument(1);
858857
String className = fromJniString(jni, name);
859858
JNIObjectHandle clazz = Support.callObjectMethodL(jni, self, bp.method, name);
@@ -914,7 +913,7 @@ private static boolean isLoadClassInvocation(JNIObjectHandle clazz, JNIMethodId
914913

915914
private static boolean findMethodHandle(JNIEnvironment jni, Breakpoint bp, InterceptedState state) {
916915
JNIObjectHandle callerClass = state.getDirectCallerClass();
917-
JNIObjectHandle lookup = getObjectArgument(0);
916+
JNIObjectHandle lookup = getReceiver();
918917
JNIObjectHandle declaringClass = getObjectArgument(1);
919918
JNIObjectHandle methodName = getObjectArgument(2);
920919
JNIObjectHandle methodType = getObjectArgument(3);
@@ -927,7 +926,7 @@ private static boolean findMethodHandle(JNIEnvironment jni, Breakpoint bp, Inter
927926

928927
private static boolean findSpecialHandle(JNIEnvironment jni, Breakpoint bp, InterceptedState state) {
929928
JNIObjectHandle callerClass = state.getDirectCallerClass();
930-
JNIObjectHandle lookup = getObjectArgument(0);
929+
JNIObjectHandle lookup = getReceiver();
931930
JNIObjectHandle declaringClass = getObjectArgument(1);
932931
JNIObjectHandle methodName = getObjectArgument(2);
933932
JNIObjectHandle methodType = getObjectArgument(3);
@@ -941,7 +940,7 @@ private static boolean findSpecialHandle(JNIEnvironment jni, Breakpoint bp, Inte
941940

942941
private static boolean bindHandle(JNIEnvironment jni, Breakpoint bp, InterceptedState state) {
943942
JNIObjectHandle callerClass = state.getDirectCallerClass();
944-
JNIObjectHandle lookup = getObjectArgument(0);
943+
JNIObjectHandle lookup = getReceiver();
945944
JNIObjectHandle receiver = getObjectArgument(1);
946945
JNIObjectHandle methodName = getObjectArgument(2);
947946
JNIObjectHandle methodType = getObjectArgument(3);
@@ -967,7 +966,7 @@ private static boolean methodMethodHandle(JNIEnvironment jni, JNIObjectHandle de
967966

968967
private static boolean findConstructorHandle(JNIEnvironment jni, Breakpoint bp, InterceptedState state) {
969968
JNIObjectHandle callerClass = state.getDirectCallerClass();
970-
JNIObjectHandle lookup = getObjectArgument(0);
969+
JNIObjectHandle lookup = getReceiver();
971970
JNIObjectHandle declaringClass = getObjectArgument(1);
972971
JNIObjectHandle methodType = getObjectArgument(2);
973972

@@ -989,7 +988,7 @@ private static JNIObjectHandle getParamTypes(JNIEnvironment jni, JNIObjectHandle
989988

990989
private static boolean findFieldHandle(JNIEnvironment jni, Breakpoint bp, InterceptedState state) {
991990
JNIObjectHandle callerClass = state.getDirectCallerClass();
992-
JNIObjectHandle lookup = getObjectArgument(0);
991+
JNIObjectHandle lookup = getReceiver();
993992
JNIObjectHandle declaringClass = getObjectArgument(1);
994993
JNIObjectHandle fieldName = getObjectArgument(2);
995994
JNIObjectHandle fieldType = getObjectArgument(3);
@@ -1004,7 +1003,7 @@ private static boolean findFieldHandle(JNIEnvironment jni, Breakpoint bp, Interc
10041003

10051004
private static boolean findClass(JNIEnvironment jni, Breakpoint bp, InterceptedState state) {
10061005
JNIObjectHandle callerClass = state.getDirectCallerClass();
1007-
JNIObjectHandle lookup = getObjectArgument(0);
1006+
JNIObjectHandle lookup = getReceiver();
10081007
JNIObjectHandle className = getObjectArgument(1);
10091008

10101009
JNIObjectHandle result = Support.callObjectMethodL(jni, lookup, bp.method, className);
@@ -1017,7 +1016,7 @@ private static boolean findClass(JNIEnvironment jni, Breakpoint bp, InterceptedS
10171016

10181017
private static boolean unreflectField(JNIEnvironment jni, Breakpoint bp, InterceptedState state) {
10191018
JNIObjectHandle callerClass = state.getDirectCallerClass();
1020-
JNIObjectHandle lookup = getObjectArgument(0);
1019+
JNIObjectHandle lookup = getReceiver();
10211020
JNIObjectHandle field = getObjectArgument(1);
10221021

10231022
JNIObjectHandle result = Support.callObjectMethodL(jni, lookup, bp.method, field);
@@ -1138,7 +1137,7 @@ private static JNIObjectHandle shouldIncludeMethod(JNIEnvironment jni, JNIObject
11381137
* register it.
11391138
*/
11401139
private static boolean serializedLambdaReadResolve(JNIEnvironment jni, @SuppressWarnings("unused") Breakpoint bp, InterceptedState state) {
1141-
JNIObjectHandle serializedLambdaInstance = getObjectArgument(0);
1140+
JNIObjectHandle serializedLambdaInstance = getReceiver();
11421141
JNIObjectHandle capturingClass = jniFunctions().getGetObjectField().invoke(jni, serializedLambdaInstance,
11431142
agent.handles().javaLangInvokeSerializedLambdaCapturingClass);
11441143

‎substratevm/src/com.oracle.svm.jvmtiagentbase/src/com/oracle/svm/jvmtiagentbase/Support.java

Lines changed: 14 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -162,14 +162,6 @@ public static boolean isSerializable(JNIEnvironment env, JNIObjectHandle seriali
162162
return jniFunctions().getIsAssignableFrom().invoke(env, serializeTargetClass, JvmtiAgentBase.singleton().handles().javaIoSerializable);
163163
}
164164

165-
public static JNIObjectHandle getCallerClass(int depth) {
166-
return getMethodDeclaringClass(getCallerMethod(depth));
167-
}
168-
169-
public static JNIObjectHandle getDirectCallerClass() {
170-
return getCallerClass(1);
171-
}
172-
173165
public static JNIMethodId getCallerMethod(int depth) {
174166
JvmtiFrameInfo frameInfo = StackValue.get(JvmtiFrameInfo.class);
175167
CIntPointer countPtr = StackValue.get(CIntPointer.class);
@@ -188,6 +180,20 @@ public static JNIObjectHandle getObjectArgument(int slot) {
188180
return handlePtr.read();
189181
}
190182

183+
/**
184+
* This method might be slightly faster than {@link #getObjectArgument}, but can only be used
185+
* for instance methods, not static methods.
186+
*/
187+
public static JNIObjectHandle getReceiver() {
188+
WordPointer handlePtr = StackValue.get(WordPointer.class);
189+
JvmtiError result = jvmtiFunctions().GetLocalInstance().invoke(jvmtiEnv(), nullHandle(), 0, handlePtr);
190+
if (result != JvmtiError.JVMTI_ERROR_NONE) {
191+
assert result != JvmtiError.JVMTI_ERROR_INVALID_SLOT : "not an instance method";
192+
return nullHandle();
193+
}
194+
return handlePtr.read();
195+
}
196+
191197
public static String getClassNameOr(JNIEnvironment env, JNIObjectHandle clazz, String forNullHandle, String forNullNameOrException) {
192198
if (clazz.notEqual(nullHandle())) {
193199
JNIObjectHandle clazzName = callObjectMethod(env, clazz, JvmtiAgentBase.singleton().handles().javaLangClassGetName);

‎substratevm/src/com.oracle.svm.jvmtiagentbase/src/com/oracle/svm/jvmtiagentbase/jvmti/JvmtiInterface.java

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -158,12 +158,14 @@ interface SetBreakpointFunctionPointer extends CFunctionPointer {
158158
JvmtiError invoke(JvmtiEnv env, JNIMethodId method, long location);
159159
}
160160

161+
/*
162+
* Note that the GetLocal*() functions execute a safepoint operation even for the current thread
163+
* and we have seen it cause serious scalability issues, presumably from the fix of JDK-8249293.
164+
*/
165+
161166
@CField("GetLocalObject")
162167
GetLocalFunctionPointer GetLocalObject();
163168

164-
@CField("GetLocalInt")
165-
GetLocalFunctionPointer GetLocalInt();
166-
167169
interface GetLocalFunctionPointer extends CFunctionPointer {
168170
@InvokeCFunctionPointer
169171
JvmtiError invoke(JvmtiEnv env, JNIObjectHandle thread, int depth, int slot, PointerBase valuePtr);

0 commit comments

Comments
 (0)