On Fri, 10 May 2024 08:55:36 GMT, Albert Mingkun Yang <[email protected]> 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
