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

List:       openjdk-serviceability-dev
Subject:    Re: RFR: JDK-8321565: [REDO] Heap dump does not contain virtual Thread stack references
From:       David Holmes <dholmes () openjdk ! org>
Date:       2023-12-13 7:24:45
Message-ID: 0l3DaiQ3yLEZlQBwYqz0VJY-xg-NKfEotrYc09b5i9M=.95163d85-032e-4a87-b94f-79c263695d99 () github ! com
[Download RAW message or body]

On Wed, 13 Dec 2023 04:59:16 GMT, Alex Menkov <amenkov@openjdk.org> wrote:

> > > So is the main thread operating in_native whilst doing the merge? I suspect the \
> > > admonition of not doing the merge at a safepoint actually meant "not a \
> > > safepoint by the VMThread" as that would cause the whole VM to pause. Even \
> > > doing it in the VMThread at all can delay the next safepoint, which does not \
> > > seem good.
> > 
> > I think the thread is in_vm.
> > Main java thread call Runtime.gc(), GC itself calls heap dumper before/after full \
> > gc. I don't know if there is a performance difference in the case between \
> > executing merge on the current thread and VMThread. \
> > HeapDumpBeforeFullGC/HeapDumpAfterFullGC/HeapDumpOnOutOfMemoryError are \
> > diagnostic flags, so I think performance is not so important here. There are \
> > plans to improve their performance by turning on parallel dump (using GC worker) \
> > - often parallel heap dump + merge takes less time than single-threaded dump (we \
> > have now). 
> > > I'm not familiar with this code in general (and only looked at this because of \
> > > the previous issue in the CI) but I'm unclear what including virtual thread \
> > > stack referencves has to do with the merging logic? I would not expect merging \
> > > to be affected by the current change.
> > 
> > This is caused by a nature of unmounted virtual threads.
> > We need to dump stack traces before we dump heap objects, but we can find \
> > unmounted virtual threads only by iterating over heap. So we need a way to add \
> > data to the beginning of the dump file while dumping heap objects. Segmented dump \
> > helps here - we have a designated segment to write stack traces.
> 
> I tested different scenarios (all I know) of heap dump:
> - via attach (jcmd);
> - JMX (HotSpotDiagnosticMXBean);
> - HeapDumpBeforeFullGC/HeapDumpAfterFullGC;
> - HeapDumpOnOutOfMemoryError.
> I see no problem with merge on the current thread.
> Only HeapDumpBeforeFullGC/HeapDumpAfterFullGC causes heap dump (and merge) at \
> safepoint. But HeapDumpBeforeFullGC/HeapDumpAfterFullGC/HeapDumpOnOutOfMemoryError \
> depend on GC and require much more testing, I think it's better to integrate this \
> fix as it is and update the way merger is executed by a separate follow-up change \
> (with cleanup of related stuff)

Okay. I'm concerned that the merge, whether by the current thread whilst in_vm, or by \
the VMThread, will either prevent the VM from going to a safepoint in a timely \
manner, or cause the safepoint to be excessively long. I'm not sure how problems in \
this area will become visible.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/17040#discussion_r1424940134


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