RFR: 8367240: Parallel: Refactor PSScavengeCLDClosure [v2]
Albert Mingkun Yang
ayang at openjdk.org
Tue Sep 9 11:48:03 UTC 2025
On Tue, 9 Sep 2025 11:15:15 GMT, Stefan Karlsson <stefank at openjdk.org> wrote:
>> Albert Mingkun Yang has updated the pull request incrementally with one additional commit since the last revision:
>>
>> review
>
> src/hotspot/share/gc/parallel/psClosure.inline.hpp line 118:
>
>> 116: if (cld->has_modified_oops()) {
>> 117: // Reset before iterating oops.
>> 118: _oop_closure._has_oop_into_young_gen = false;
>
> I wonder if this wouldn't be cleaner if you set up a new `PSScavengeFromCLDClosure` here instead. Then you wouldn't have to explicitly reset _has_oop_into_young_gen here.
>
> if (cld->has_modified_oops()) {
> PSScavengeFromCLDClosure oop_closure(_pm);
>
> // Clean the cld since we're going to scavenge all the metadata.
> cld->oops_do(&oop_closure, ClassLoaderData::_claim_none, /*clear_modified_oops*/true);
>
> if (oop_closure.has_oops_into_young_gen()) {
> cld->record_modified_oops();
> }
> }
>
>
> Maybe you don't have to hide `PSScavengeFromCLDClosure` inside `PSScavengeCLDClosure`. I find it more distracting that what we had before. If you don't agree with that, maybe the PSScavengeCLDClosure name could be changed so that we don't repeat the PSScavenge prefix for the internal class.
I have simplified the class name and moved it inside the `do_cld` method.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/27165#discussion_r2333211266
More information about the hotspot-gc-dev
mailing list