[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-serviceability-dev
Subject:    Re: RFR: 8324066: "clhsdb jstack" should not by default scan for j.u.c locks because it can be very 
From:       Chris Plummer <cjplummer () openjdk ! org>
Date:       2024-01-31 23:07:16
Message-ID: T2jz7IinHBaSQEFhVqsG2QtZKpCRdIh4ppGyISAaFUY=.c55b8e87-4db3-4b6b-8486-40534685cc2d () github ! com
[Download RAW message or body]

> 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().

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

Changes:
  - all: https://git.openjdk.org/jdk/pull/17479/files
  - new: https://git.openjdk.org/jdk/pull/17479/files/6c24a9a2..929de70f

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk&pr=17479&range=03
 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=17479&range=02-03

  Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod
  Patch: https://git.openjdk.org/jdk/pull/17479.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/17479/head:pull/17479

PR: https://git.openjdk.org/jdk/pull/17479


[prev in list] [next in list] [prev in thread] [next in thread]