On Mon, 22 Mar 2021 14:22:36 GMT, Daniel D. Daugherty <[email protected]> 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
