RFR: 8360024: Reorganize GC VM operations and implement is_gc_operation [v2]

[Thomas Schatzl]

On Fri, 20 Jun 2025 09:38:56 GMT, Jonas Norlinder <duke at openjdk.org> wrote:

>> [JDK-8359110](https://bugs.openjdk.org/browse/JDK-8359110) introduces a need to be able to categorize VM operations to be able to track GC activity in the VM thread for CPU time tracking. This patch will reorganize the shared GC VM operations in gcVMOperations.hpp to make a distinction between GC operations and serviceability/CDS etc. operations performed in the VM thread for the GC.
>> 
>> Additionally the ability to query `is_gc_operation()` is introduced in the base class `VM_Operation` and overridden in appropriate VM operations. This provides a clear and efficient interface to query the category of a VM operation.
>> 
>> Proposed changes in the shared class hierarchy:
>> 
>> 
>> //  VM_Operation
>> //    VM_Heap_Sync_Operation
>> //      VM_GC_Operation
>> //        VM_GC_Collect_Operation
>> //          VM_SerialGCCollect
>> //          VM_ParallelGCCollect
>> //          VM_CollectForAllocation
>> //            VM_SerialCollectForAllocation
>> //            VM_ParallelCollectForAllocation
>> //          VM_CollectForMetadataAllocation
>> //        VM_GC_Service_Operation
>> //          VM_GC_HeapInspection
>> //      VM_Verify
>> //      VM_PopulateDynamicDumpSharedSpace
>> //      VM_PopulateDumpSharedSpace
>> 
>> 
>> Renaming `VM_GC_Sync_Operation` to `VM_Heap_Sync_Operation` to make it clear that not all sub-classes is related to GC (shout-out to @kstefanj / @stefank for that suggestion). While `VM_GC_HeapInspection` is a serviceability operation it attempts to trigger a collection, hence it is put underneath `VM_GC_Collect_Operation->VM_GC_Service_Operation`. However only classes that inherits from `VM_GC_Collect_Operation` are considered to return `true` for `is_gc_operation()` as `VM_GC_HeapInspection` is considered to not belong to "GC activity".
>> 
>> ZGC and Shenandoah do not use sub-classing of `VM_Heap_Sync_Operation`, but their respective base classes and special classes for GC activity are marked as returning `true` for `is_gc_operation()`.
>> 
>> The only `is_XX_operation` that is specified in `VM_Operation` is `is_gc_operation` to not clutter with cases that are not used (`is_gc_operation` will be used by JDK-8359110). That being said, these behavioral queries may be extended in case we want to explore CPU time tracking beyond the GC component.
>> 
>> _Note: PR for JDK-8359110 already contains a proposal for `is_gc_operation` but it was deemed that it was more appropriate to bring along that change during reorganization of GC VM operations since implementing it...
>
> Jonas Norlinder has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Re-align arguments

lgtm, but since the comment typos cause re-reviews, requesting changes.

src/hotspot/share/gc/shared/gcVMOperations.hpp line 47:

> 45: //          VM_CollectForAllocation
> 46: //            VM_SerialCollectForAllocation
> 47: //            VM_ParallelCollectForAllocation

Suggestion:

//          e.g. VM_SerialGCCollector
//          VM_CollectForAllocation
//            e.g. VM_SerialCollectForAllocation


This is not a real suggestion, but this hierarchy isn't complete, and I am not sure we should make it complete. Since we already skip Z/G1/Shenandoah operations here, I do not really see that Serial/Parallel specific operations should be here either.

Or maybe explicitly state Serial/Parallel vm ops as examples here as suggested above, but that does not look right either.

However this is kind of unrelated to this PR :)

src/hotspot/share/runtime/vmOperation.hpp line 164:

> 162: 
> 163:   // VMOp_Type may belong to a category of operation
> 164:   // Override is_XX_operation() appropriately in subclasses

Suggestion:

  // VMOp_Type may belong to a category of the operation.
  // Override is_XX_operation() appropriately in subclasses.

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

Changes requested by tschatzl (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/25896#pullrequestreview-2949032479
PR Review Comment: https://git.openjdk.org/jdk/pull/25896#discussion_r2160960642
PR Review Comment: https://git.openjdk.org/jdk/pull/25896#discussion_r2160950023