RFR: 8331557: Serial: Refactor SerialHeap::do_collection [v6]

[Ivan Walulya]

On Fri, 10 May 2024 08:55:36 GMT, Albert Mingkun Yang <ayang at openjdk.org> wrote:

>> It's probably easier to read the new code directly. The two classes in `serialVMOperations` serve as entrance points to invoke young/full GCs. Some previously hidden decisions are made more obvious, e.g. if a young-gc fails (or will probablly fail), fallback to full-gc.
>> 
>> Additionally, `StatRecord` is removed, because this kind of info-aggregation should be done outsite VM (by third-party tool).
>> 
>> Test: tier1-6
>
> 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 seven commits:
> 
>  - Merge branch 'master' into s1-do-collect
>  - review
>  - Merge branch 'master' into s1-do-collect
>  - merge
>  - review
>  - Merge branch 'master' into s1-do-collect
>  - s1-do-collect

Changes requested by iwalulya (Reviewer).

src/hotspot/share/gc/serial/serialHeap.cpp line 557:

> 555:     return result;
> 556:   }
> 557: 

Would be nice to add a comment here to indicate that the previous collection could have shrunk the heap.

src/hotspot/share/gc/serial/serialHeap.cpp line 714:

> 712: 
> 713: void SerialHeap::do_full_collection_no_gc_locker(bool clear_all_soft_refs) {
> 714:   IsSTWGCActiveMark gc_active_mark;

`IsSTWGCActiveMark active_gc_mark;`in`do_young_collection_no_gc_locker`, just choose one and be consistent with it

src/hotspot/share/gc/serial/serialHeap.cpp line 907:

> 905: 
> 906: void SerialHeap::print_tracing_info() const {
> 907:  // Nothing

What is the `Nothing` supposed to convey here?

src/hotspot/share/gc/serial/serialHeap.hpp line 117:

> 115:   void do_full_collection_no_gc_locker(bool clear_all_soft_refs);
> 116: 
> 117:   void collect_at_safepoint_no_gc_locker(bool full);

I am not very convinced by the naming of the methods with the "no_gc_locker" constraint.  But I guess it is following same convention as "*at_safepoint" method naming.

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

PR Review: https://git.openjdk.org/jdk/pull/19056#pullrequestreview-2062384283
PR Review Comment: https://git.openjdk.org/jdk/pull/19056#discussion_r1604380445
PR Review Comment: https://git.openjdk.org/jdk/pull/19056#discussion_r1604389765
PR Review Comment: https://git.openjdk.org/jdk/pull/19056#discussion_r1604390627
PR Review Comment: https://git.openjdk.org/jdk/pull/19056#discussion_r1604388902