While working on JDK-8256641 we found that the code could be brushed up a bit:
- improve handling of VM_GC_Operations that do not participate in the operation skipping protocol like VM_GC_HeapInspection: either let them inherit from VM_GC_Sync_Operation (and add the check that GC must have been initialized when calling this, see VM_GC_Operation::doit_prologue()), or try to change the order of constructor parameters so that not every one of those needs to add an "this is ignored" comment
- maybe GCCause can be made an enum class
- some of those operation skipping VM_GC_Operations seem to duplicate the _full member
- some of those operation skipping VM_GC_Operations pass a GCCause in the VM_GC_Operation but then do not use the _gc_cause member
- use initializer lists
- documentation/indentation style (single space before visibility modifier, column aligned member names, ...) could be brushed up
- not all VM_GC_Operations are in that file (but maybe ignore this)
- those not operation-skipping VM_Operations kind of have the same helper method to do the gc e.g.
bool VM_GC_HeapInspection::collect() {
if (GCLocker::is_active()) {
return false;
}
Universe::heap()->collect_as_vm_thread(GCCause::_heap_inspection);
return true;
}
[...]
Universe::heap()->ensure_parsability(...);
if (!collect()) {
log_warning(gc)(...
}
Maybe there could be a base class for this case unifying this.
Probably other issues. Create new issues as appropriate.