Hi Daniil,
I have several quick comments.
The indent in the hotspot c/c++ files has to be 2, not 4.
https://cr.openjdk.java.net/~dtitov/8185005/webrev.01/src/hotspot/share/runtime/threadSMR.cpp.frames.html
614 JavaThread* ThreadsList::find_JavaThread_from_java_tid(jlong java_tid)
const {
615 JavaThread* java_thread = ThreadTable::find_thread(java_tid);
616 if (java_thread == NULL && java_tid == PMIMORDIAL_JAVA_TID) {
617 // ThreadsSMRSupport::add_thread() is not called for the primordial
618 // thread. Thus, we find this thread with a linear search and add it
619 // to the thread table.
620 for (uint i = 0; i < length(); i++) {
621 JavaThread* thread = thread_at(i);
622 if (is_valid_java_thread(java_tid,thread)) {
623 ThreadTable::add_thread(java_tid, thread);
624 return thread;
625 }
626 }
627 } else if (java_thread != NULL && is_valid_java_thread(java_tid,
java_thread)) {
628 return java_thread;
629 }
630 return NULL;
631 }
632 bool ThreadsList::is_valid_java_thread(jlong java_tid, JavaThread*
java_thread) {
633 oop tobj = java_thread->threadObj();
634 // Ignore the thread if it hasn't run yet, has exited
635 // or is starting to exit.
636 return (tobj != NULL && !java_thread->is_exiting() &&
637 java_tid == java_lang_Thread::thread_id(tobj));
638 }
615 JavaThread* java_thread = ThreadTable::find_thread(java_tid); I'd
suggest to rename find_thread() tofind_thread_by_tid().
A space is missed after the comma:
622 if (is_valid_java_thread(java_tid,thread)) {
An empty line is needed before L632.
The name 'is_valid_java_thread' looks wrong (or confusing) to me.
Something like 'is_alive_java_thread_with_tid()' would be better.
It'd better to list parameters in the opposite order.
The call to is_valid_java_thread() is confusing:
627 } else if (java_thread != NULL && is_valid_java_thread(java_tid,
java_thread)) {
Why would the call ThreadTable::find_thread(java_tid) return a
JavaThread with an unmatched java_tid?
Thanks,
Serguei
On 6/28/19 3:31 PM, Daniil Titov wrote:
Please review the change that improves performance of ThreadMXBean MXBean
methods returning the
information for specific threads. The change introduces the thread table that
uses ConcurrentHashTable
to store one-to-one the mapping between the thread ids and JavaThread objects
and replaces the linear
search over the thread list in ThreadsList::find_JavaThread_from_java_tid(jlong
tid) method with the lookup
in the thread table.
Testing: Mach5 tier1,tier2 and tier3 tests successfully passed.
Webrev: https://cr.openjdk.java.net/~dtitov/8185005/webrev.01/
Bug: https://bugs.openjdk.java.net/browse/JDK-8185005
Thanks!
Best regards,
Daniil
