RFR: 8270009: Factor out and shuffle methods in G1CollectedHeap::do_collection_pause_at_safepoint_helper

Albert Mingkun Yang ayang at openjdk.java.net
Mon Jul 12 07:23:52 UTC 2021

On Fri, 9 Jul 2021 14:40:50 GMT, Thomas Schatzl <tschatzl at openjdk.org> wrote:

> Hi all,
>   can I get reviews for this change that factors out a few methods from `do_collection_pause_at_safepoint_helper` and reshuffles code from `gc_prologue`/`gc_epilogue` to be able to put those into parallel phase(s) later.
> Testing: tier1-5
> Thanks,
>   Thomas

Only minor comments regarding certain names.

src/hotspot/share/gc/g1/g1CollectedHeap.cpp line 2990:

> 2988: }
> 2989: 
> 2990: void G1CollectedHeap::set_default_active_worker_threads(){

The name, `set_default_active_worker_threads`, is quite generic, suggesting it's called from multiple places. However, it's actually only for the incremental evacuation pause, right? If so, it's probably clearer to include "evacuation" in the name, IMO.

src/hotspot/share/gc/g1/g1CollectedHeap.cpp line 3851:

> 3849:   prepare_tlabs_for_mutator();
> 3850: 
> 3851:   gc_epilogue(false);

I wonder if `gc_epilogue` could be made more explicit about what it includes, like its neighbors.


Marked as reviewed by ayang (Committer).

PR: https://git.openjdk.java.net/jdk/pull/4744

More information about the hotspot-gc-dev mailing list