RFR: 8262881: port JVM/DI tests from JDK-4413752 to JVM/TI [v2]

[Serguei Spitsyn]

On Mon, 22 Mar 2021 14:22:36 GMT, Daniel D. Daugherty <dcubed at openjdk.org> wrote:

>> 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
>
> @robehn - Thanks for the review and for the suggested improvements to the tests.
> It will take me a bit of time to make and test these suggestions.

Hi Dan,
> I figured you would enjoy this 20 year old blast from the past!
Yes, of course, but these tests look like they have been written today! :)

Thank you for making changes!
I've just noticed one minor issue:

libSuspendWithObjectMonitorEnter.cpp
libSuspendWithObjectMonitorWait.cpp:
The static variables below are not used and can be removed:
 32 static jrawMonitorID threadLock = NULL;
 33 static char threadLockName[] = "threadLock";

Thanks,
Serguei

-------------

PR: https://git.openjdk.java.net/jdk/pull/2899