battleblow / openjdk-jdk11u

BSD port of OpenJDK 11
GNU General Public License v2.0
9 stars 8 forks source link

Adjust BSD os::thread_cpu_time() implementation: #41

Closed bsdkurt closed 5 years ago

bsdkurt commented 5 years ago

Notes: On OpenBSD:

gmake run-test-only TEST="runtime/jni/terminatedThread/TestTerminatedThread.java"

Crashed the jvm by calling pthread_getcpuclockid on a terminated thread. It appears there isn't a safe way to check if a thread is terminated in the JVM and the pthread_getcpuclockid implementation doesn't work when the caller wants just user time anyway so I coded up a alternate solution.

Making getrusage(RUSAGE_THREAD) preferred for *BSD for the the current thread wasn't strictly necessary but I think makes sense if FreeBSD and NetBSD provide fully functional implementations similar to what I have done.

bsdkurt commented 5 years ago

@battleblow I decided to not make the change to RUSAGE_THREAD and leave it the way it was. NetBSD doesn't have RUSAGE_THREAD, so if you convert the FreeBSD implementation over to KERN_PROC_PID w/threads as well it can be removed then.

Here are the relevant tests for testing:

gmake run-test-only TEST="hotspot/jtreg/runtime/jni/terminatedThread/TestTerminatedThread.java jdk/jfr/event/runtime/TestThreadCpuTimeEvent.java"

okay to merge the current version?

battleblow commented 5 years ago

@battleblow I decided to not make the change to RUSAGE_THREAD and leave it the way it was. NetBSD doesn't have RUSAGE_THREAD, so if you convert the FreeBSD implementation over to KERN_PROC_PID w/threads as well it can be removed then.

Here are the relevant tests for testing:

gmake run-test-only TEST="hotspot/jtreg/runtime/jni/terminatedThread/TestTerminatedThread.java jdk/jfr/event/runtime/TestThreadCpuTimeEvent.java"

okay to merge the current version?

Yes, please go ahead and merge. I think it is reasonable to leave it as is for now. Ideally we'd be able to instrument this and see how often each version is called, but I can't think of an easy way to do it.