RFR: 8299426: Heap dump does not contain virtual Thread stack references

[Alex Menkov]

On Thu, 16 Nov 2023 02:30:23 GMT, Yi Yang <yyang at openjdk.org> wrote:

>> The change impelements dumping of unmounted virtual threads data (stack traces and stack references).
>> Unmounted vthreads can be detected only by iterating over the heap, but hprof stack trace records (HPROF_FRAME/HPROF_TRACE) should be written before HPROF_HEAP_DUMP/HPROF_HEAP_DUMP_SEGMENT.
>> HeapDumper supports segment dump (parallel dump to separate files with subsequent file merge outside of safepoint), the fix switches HeapDumper  to always use segment dump: 1st segment contains only non-heap data, other segments are used for dumping heap objects. For serial dumping single-threaded dumping is performed, but 2 segments are created anyway.
>> When HeapObjectDumper detects unmounted virtual thread, it writes HPROF_FRAME/HPROF_TRACE records to the 1st segment ("global writer"), and writes thread object (HPROF_GC_ROOT_JAVA_FRAME) and stack references (HPROF_GC_ROOT_JAVA_FRAME/HPROF_GC_ROOT_JNI_LOCAL) to the HeapObjectDumper segment.
>> As parallel dumpers may write HPROF_FRAME/HPROF_TRACE concurrently and VMDumper needs to write non-heap data before heap object dumpers can write virtual threads data, writing to global writer is protected with DumperController::_global_writer_lock.
>> 
>> Testing: run tests which perform heap dump (in different scenarios):
>>  - test/hotspot/jtreg/serviceability
>>  - test/hotspot/jtreg/runtime/ErrorHandling
>>  - test/hotspot/jtreg/gc/epsilon
>>  - test/jdk/sun/tools/jhsdb
>
> src/hotspot/share/services/heapDumper.cpp line 1896:
> 
>> 1894: 
>> 1895:  public:
>> 1896:   HeapObjectDumper(AbstractDumpWriter* writer, UnmountedVThreadDumper* vthread_dumper)
> 
> Maybe we can pass global writer to HeapObjectDumper and thus simplify the implementation? We can remove `class VM_HeapDumper : public VM_GC_Operation, public WorkerTask, public UnmountedVThreadDumper {` and `UnmountedVThreadDumper` then.

This was an initial plan, but it would require more refactoring (dumper for unmounted vthreads needs a way to lock global writer, so need to move DumperController declaration and definition or add new 'class GlobalDumpWriter: public DumpWriter' and implement the lock there). I prefer to make functional changes here with minimum refactoring to simplify review and do refactoring in follow-up fixes

> src/hotspot/share/services/heapDumper.cpp line 2024:
> 
>> 2022: char* DumpMerger::get_writer_path(const char* base_path, int seq) {
>> 2023:   // calculate required buffer size
>> 2024:     size_t buf_size = strlen(base_path)
> 
> Format

fixed

> src/hotspot/share/services/heapDumper.cpp line 2411:
> 
>> 2409: 
>> 2410:   WorkerThreads* workers = ch->safepoint_workers();
>> 2411:   update_parallel_dump_count(workers);
> 
> Maybe consolidate them together
> 
>   update_parallel_dump_count(workers);
> 
> 
> =>
> 
> prepare_parallel_dump(workers){
>  ...  // copy from update_parallel_dump_count
>  _dumper_controller = new (std::nothrow) DumperController(_num_dumper_threads);
> }

Done

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

PR Review Comment: https://git.openjdk.org/jdk/pull/16665#discussion_r1396386291
PR Review Comment: https://git.openjdk.org/jdk/pull/16665#discussion_r1396359118
PR Review Comment: https://git.openjdk.org/jdk/pull/16665#discussion_r1396359593