On Fri, 19 Mar 2021 20:16:29 GMT, Daniel D. Daugherty <[email protected]>
wrote:
>> Hi Dan,
>> It is interesting how much these tests were changed when ported.
>> A couple of indent-related comments.
>> There are incorrect indents in the following lines in all .cpp files:
>> ```
>> 68 Java_SuspendWithObjectMonitorEnter_GetResult(JNIEnv *env, jclass cls) {
>> 69 return iGlobalStatus;
>> 70 }
>> 72 JNIEXPORT void JNICALL
>> 73 Java_SuspendWithObjectMonitorEnter_SetPrintDebug(JNIEnv *env, jclass
>> cls) {
>> 74 printdebug = 1;
>> 75 }
>> 97 Java_SuspendWithObjectMonitorEnterWorker_GetPrintDebug(JNIEnv *env,
>> jclass cls) {
>> 98 return printdebug;
>> 99 }
>>
>> Thanks,
>> Serguei
>
> @sspitsyn - Thanks for the review! I figured you would enjoy this 20 year old
> blast from the past! I'm tempted to ping @karenkinnear just to see if she'll
> remember these tests!
>
> I'll fix the indents in the .cpp files. Right now I'm using these tests to
> shake
> out @robehn's work on "JDK-8257831 Suspend with handshakes".
Hi Dan,
If you change the native methods, e.g. :
native static void RawMonitorEnter(int id);
To something like:
native static int RawMonitorEnter();
You can then handle the jvmti error in the Java code, so you don't need to pass
down the 'id' of the thread.
You then remove all debug code from the C-code agent, which removes some native
methods also.
Which also leads to that you don't need the thread 'id', instead you can just
use the thread name, which removes some Java code.
Also you shouldn't catch the UnsatisfiedLinkError, as docs also says: "An Error
is a subclass of Throwable that indicates serious problems that a reasonable
application should not try to catch."
You also have a lot of code for argument parsing which is not used by the test
inside the test methods.
Can you either remove that or if you want it put it in a separate class/method,
so e.g. "doWork()" takes parsed arguments instead.
Thanks, Robbin
-------------
PR: https://git.openjdk.java.net/jdk/pull/2899
