On Wed, 31 Jan 2024 23:07:16 GMT, Chris Plummer <cjplum...@openjdk.org> wrote:

>> I noticed that "clhsdb jstack" seemed to hang when I attached to process 
>> with a somewhat large heap. It had taken over 10 minutes when I finally 
>> decided to have a look at the SA process (using bin/jstack), which came up 
>> with the following in the stack:
>> 
>> 
>> at 
>> sun.jvm.hotspot.oops.ObjectHeap.iterateObjectsOfKlass([jdk.hotspot.agent@23-internal](jdk.hotspot.agent@23-internal)/ObjectHeap.java:117)
>> at 
>> sun.jvm.hotspot.runtime.ConcurrentLocksPrinter.fillLocks([jdk.hotspot.agent@23-internal](jdk.hotspot.agent@23-internal)/ConcurrentLocksPrinter.java:70)
>> at 
>> sun.jvm.hotspot.runtime.ConcurrentLocksPrinter.<init>([jdk.hotspot.agent@23-internal](jdk.hotspot.agent@23-internal)/ConcurrentLocksPrinter.java:36)
>> at 
>> sun.jvm.hotspot.tools.StackTrace.run([jdk.hotspot.agent@23-internal](jdk.hotspot.agent@23-internal)/StackTrace.java:71)
>> 
>> 
>> This code is meant to print information about j.u.c. locks. It it works by 
>> searching the entire java heap for instances of AbstractOwnableSynchronizer. 
>> This is a very expensive operation because it means not only does SA need to 
>> read in the entire java heap, but it needs to create a Klass mirror for 
>> every heap object so it can determine its type.
>> 
>> Our testing doesn't seem to run into this slowness problem because "clhsdb 
>> jstack" only iterates over the heap if AbstractOwnableSynchronizer has been 
>> loaded, which it won't be if no j.u.c. locks have been created. This seems 
>> to be the case wherever we use "clhsdb jstack" in testing. We do have some 
>> tests that likely result in j.u.c locks, but they all use "jhsdb jstack", 
>> which doesn't have this issue (it requires use of the --locks argument to 
>> enable printing of j.u.c locks).
>> 
>> This issue was already called out in 
>> [JDK-8262098](https://bugs.openjdk.org/browse/JDK-8262098). For this CR I am 
>> proposing that "clhsdb jstack" not include j.u.c lock scanning by default, 
>> and add the -l argument to enable it. This will make it similar to 
>> bin/jstack, which also has a -l argument, and to "clhsdb jstack", which has 
>> a --locks argument (which maps internally to the Jstack.java -l argument).
>> 
>> The same changes are also being made to "clhsdb pstack".
>> 
>> Tested with all tier1, tier2, and tier5 svc tests.
>
> Chris Plummer has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   simplify regex passed to String.split().

Thanks for the reviews Kevin and Alex!

-------------

PR Comment: https://git.openjdk.org/jdk/pull/17479#issuecomment-1922065831

Reply via email to