RFR: 8185005: Improve performance of ThreadMXBean.getThreadInfo(long ids[], int maxDepth)

[serguei.spitsyn at oracle.com]

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
>
>
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.java.net/pipermail/serviceability-dev/attachments/20190628/60aa088d/attachment.html>