RFR: 8307348 - Parallelize heap walk for ObjectCount(AfterGC) JFR event collection [v5]

[olivergillespie]

On Fri, 5 May 2023 11:35:40 GMT, Albert Mingkun Yang <ayang at openjdk.org> wrote:

>> olivergillespie has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains six additional commits since the last revision:
>> 
>>  - Rework
>>    
>>    Pass workers through from caller. Move some of the specific handling
>>    to the VM Op for heap inspection.
>>  - Merge remote-tracking branch 'origin/master' into 8307348
>>  - Use ParallelGCThreads instead of active_workers
>>  - Fix compile error
>>    
>>    ```
>>     === Output from failing command(s) repeated here ===
>>    * For target hotspot_variant-server_libjvm_objs_gcTrace.o:
>>    /home/runner/work/jdk/jdk/src/hotspot/share/gc/shared/gcTrace.cpp: In member function 'void GCTracer::report_object_count_after_gc(BoolObjectClosure*)':
>>    /home/runner/work/jdk/jdk/src/hotspot/share/gc/shared/gcTrace.cpp:114:48: error: invalid use of incomplete type 'class CollectedHeap'
>>      114 |       WorkerThreads* workers = Universe::heap()->safepoint_workers();
>>          |                                                ^~
>>    In file included from /home/runner/work/jdk/jdk/src/hotspot/share/gc/shared/gcTrace.cpp:35:
>>    /home/runner/work/jdk/jdk/src/hotspot/share/memory/universe.hpp:42:7: note: forward declaration of 'class CollectedHeap'
>>       42 | class CollectedHeap;
>>          |       ^~~~~~~~~~~~~
>>    
>>    * All command lines available in /home/runner/work/jdk/jdk/build/linux-x64/make-support/failure-logs.
>>    === End of repeated output ===
>>    ```
>>  - Fix compile error
>>  - 8307348 - Parallelize heap walk for ObjectCount(AfterGC) JFR event collection
>
> src/hotspot/share/gc/shared/gcVMOperations.cpp line 174:
> 
>> 172:       // Can't run with more threads than provided by the WorkerThreads.
>> 173:       const uint capped_parallel_thread_num = MIN2(_parallel_thread_num, workers->max_workers());
>> 174:       WithActiveWorkers with_active_workers(workers, capped_parallel_thread_num);
> 
> `WithActiveWorkers` is an stack-obj, so it must be in the same scope as using workers.

Oh, good catch. So can I do something like:


HeapInspection inspect;
if (workers != nullptr) {
    // The GC provided a WorkerThreads to be used during a safepoint.
    // Can't run with more threads than provided by the WorkerThreads.
    const uint capped_parallel_thread_num = MIN2(_parallel_thread_num, workers->max_workers());
    WithActiveWorkers with_active_workers(workers, capped_parallel_thread_num);
    inspect.heap_inspection(_out, workers);
} else {
    inspect.heap_inspection(_out, nullptr);
}


?

(I'm not familiar with the exact workings of StackObj, sorry)

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

PR Review Comment: https://git.openjdk.org/jdk/pull/13774#discussion_r1185998092