During the code review for:

JDK-8273107 RunThese24H times out with "java.lang.management.ThreadInfo.getLockName()" is null

[~rehn] noticed that InflatedMonitorsClosure can be inefficient.

Here's the comments from that review:

https://github.com/openjdk/jdk18/pull/25#discussion_r769378235

and pasted here also:

src/hotspot/share/services/threadService.cpp
      // Get the ObjectMonitors locked by this thread (if any):
      ObjectMonitorsHashtable::PtrList* list = table->get_entry(_thread);
      if (list != nullptr) {
        ObjectSynchronizer::monitors_iterate(&imc, list, _thread);
  
Member
@robehn robehn 3 days ago
The InflatedMonitorsClosure walks the stack until object is found with this method:
ThreadStackTrace::is_owned_monitor_on_stack(oop object)
for every object...

If you instead just collect all locks on stack with one pass you don't have to walk the stack over and over, which should be major speedup.
  
Member
Author
@dcubed-ojdk dcubed-ojdk 2 days ago
Sounds like an interesting RFE, but I'd prefer that be investigated
separately from this bug.
  
Member
Author
@dcubed-ojdk dcubed-ojdk 2 days ago
Clarification: ThreadStackTrace::is_owned_monitor_on_stack() is not actually
walking the (JavaThread's) stack, but it is walking the collection of
StackFrameInfo objects that was gathered earlier in
ThreadStackTrace::dump_stack_at_safepoint(). Adding a gather-locked-monitor
helper object to the "add_stack_frame()" call would provide an easy hook and
then InflatedMonitorsClosure could use the new helper object. Something like
that anyway...
  
Member
@robehn robehn 2 days ago
Yes, sorry, we loop the stack frame info for every object. I.e. a loop in a loop which is bad for time-complexity.
If the we created a sorted list from the frame info, we can binary search it instead.
Since a thread often only have at max a few locks, but could have hundreds of frames, just to be able skip looping over all frames without locks would be very beneficial.