RFR: 8367240: Parallel: Refactor PSScavengeCLDClosure [v2]

Stefan Karlsson stefank at openjdk.org
Tue Sep 9 15:20:05 UTC 2025


On Tue, 9 Sep 2025 11:48:01 GMT, Albert Mingkun Yang <ayang at openjdk.org> wrote:

>> Refactor `PSScavengeCLDClosure` and its "enclosing" `PSScavengeFromCLDClosure` so that all CLD logic is encapsulated in the CLD closure, without polluting the oop closure.
>> 
>> Test: tier1-3
>
> Albert Mingkun Yang has updated the pull request incrementally with one additional commit since the last revision:
> 
>   review

Changes requested by stefank (Reviewer).

src/hotspot/share/gc/parallel/psClosure.inline.hpp line 94:

> 92: 
> 93:     // Scavenges a single oop in a ClassLoaderData.
> 94:     class CLDOopClosure : public OopClosure {

This makes the do_cld function much harder to read, IMHO. So, three alternatives:
1) Keep the class outside PSScavengeCLDClosure, just like it was before this patch.
2) Have the class inside PSScavengeCLDClosure, but not in a function
3) Have the class inside a function inside PSScavengeCLDClosure.

I prefer (1) because it allows you to look at PSScavengeCLDClosure without having to see the details about the second class. I'm OK with (2) and like that it encapsulates the other class. I don't think we should do (3) because it significantly hurts the readability of do_cld.

src/hotspot/share/gc/parallel/psClosure.inline.hpp line 99:

> 97:     public:
> 98:       // Records whether this CLD contains oops pointing into young-gen after scavenging.
> 99:       bool _has_oop_into_young_gen;

Comment says oops but name says oop. Maybe use _has_oops_into_young_gen?

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

PR Review: https://git.openjdk.org/jdk/pull/27165#pullrequestreview-3202095030
PR Review Comment: https://git.openjdk.org/jdk/pull/27165#discussion_r2333984554
PR Review Comment: https://git.openjdk.org/jdk/pull/27165#discussion_r2333965840


More information about the hotspot-gc-dev mailing list