On Mon, 8 Jul 2024 16:31:43 GMT, Albert Mingkun Yang <ay...@openjdk.org> wrote:
>> Similar cleanup as https://github.com/openjdk/jdk/pull/19056 but in >> Parallel. As a result, the corresponding code in `SerialHeap` and >> `ParallelScavengeHeap` share much similarity. >> >> The easiest way to review is to start from these two VM operations, >> `VM_ParallelCollectForAllocation` and `VM_ParallelGCCollect` and follow the >> new code directly, where one can see how allocation-failure triggers various >> GCs with different collection efforts. >> >> Test: tier1-6; perf-neural for dacapo, specjvm2008, specjbb2015 and >> cachestresser. > > Albert Mingkun Yang has updated the pull request with a new target base due > to a merge or a rebase. The pull request now contains one commit: > > pgc-vm-operation I really like this refactor, that brings parallel close to other GCs. Just a few nits, otherwise, LGTM src/hotspot/share/gc/parallel/parallelScavengeHeap.cpp line 273: > 271: > 272: bool is_tlab = false; > 273: return mem_allocate_work(size, is_tlab, > gc_overhead_limit_was_exceeded); Suggest: `return mem_allocate_work(size, false /* is_tlab */, gc_overhead_limit_was_exceeded);` src/hotspot/share/gc/parallel/parallelScavengeHeap.cpp line 478: > 476: > 477: const bool clear_all_soft_refs = true; > 478: do_full_collection_no_gc_locker(clear_all_soft_refs); Suggest: not define `const bool clear_all_soft_refs = true;` and do `do_full_collection_no_gc_locker(true /* clear_all_soft_refs */);` instead src/hotspot/share/gc/parallel/psVMOperations.cpp line 68: > 66: > 67: GCCauseSetter gccs(heap, _gc_cause); > 68: heap->try_collect_at_safepoint(is_cause_full(_gc_cause)); can be simplified to `heap->try_collect_at_safepoint(_full);` ------------- Marked as reviewed by zgu (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/20077#pullrequestreview-2166570482 PR Review Comment: https://git.openjdk.org/jdk/pull/20077#discussion_r1670678592 PR Review Comment: https://git.openjdk.org/jdk/pull/20077#discussion_r1671439533 PR Review Comment: https://git.openjdk.org/jdk/pull/20077#discussion_r1672170373
