On Wed, 13 Dec 2023 01:38:03 GMT, Alex Menkov <amen...@openjdk.org> wrote:
>>> I.e. merge is performed on main java thread, but VM is at safepoint >> >> 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'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. > >> 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) ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/17040#discussion_r1424848688
