On Mon, 11 Dec 2023 06:05:52 GMT, David Holmes <dhol...@openjdk.org> wrote:
>> Original fix for JDK-8299426 (Heap dump does not contain virtual Thread
>> stack references, #16665) caused failures of new test (added while #16665
>> was under review):
>> test/hotspot/jtreg/compiler/c2/TestReduceAllocationAndHeapDump.java in many
>> tears and was reverted.
>>
>> Segmented heap dump assumes "merge" stage is executed outside of safepoint
>> (to not block the VM), but heap dump may happen during safepoint (and
>> TestReduceAllocationAndHeapDump.java test provoke the case).
>> The change contains original fix for JDK-8299426 ("[original
>> fix](https://github.com/openjdk/jdk/commit/bdbf768eafa86e0007aca4188e0567693afe9071)")
>> and removes asserts from HeapMerger ([allow heapdump in
>> safepoints](https://github.com/openjdk/jdk/commit/44670ca4bf55dd2a5f1f44686758844aed68937e)).
>>
>> Run tier1-3 and heapdump-related tests.
>
> src/hotspot/share/services/heapDumper.cpp line 1984:
>
>> 1982: // user space.
>> 1983: void DumpMerger::merge_file(char* path) {
>> 1984: assert(!SafepointSynchronize::is_at_safepoint(), "merging happens
>> outside safepoint");
>
> This might fix the failure but why is this restriction in place to begin
> with? This seems like a very expensive operation to be performing by the
> VMThread during a safepoint!
Main idea of segmented heap dump feature (JDK-8306441, #13667) is to divide
heap dump process to 2 phases - dump to segment files (this phase is executed
at safepoint) and merge of the segment files (this one is supposed to be
executed outside of safepoint, so VM is not blocked in this phase). And this
asserts were added to ensure it works as desired.
I think merge stage can be performed on the current thread without VMOperation,
but I don't know all possible consequences of the change.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/17040#discussion_r1423261940
