RFR: 8295871: G1: Use different explicit claim marks for CLDs
Stefan Johansson
sjohanss at openjdk.org
Tue Nov 8 14:03:26 UTC 2022
On Fri, 4 Nov 2022 14:46:24 GMT, Thomas Schatzl <tschatzl at openjdk.org> wrote:
> Hi all,
>
> can I have reviews for this follow-up to [JDK-8295118](https://bugs.openjdk.org/browse/JDK-8295118) that removes the need to clear CLD claim marks for every full gc phase by using different claim values for the different phases.
>
> Some comments:
> * I used new g1 specific claim values instead of overloading the existing ones, which is imho clearer. I am open to better names, but something like `_claim_strong_2/3` seemed too cryptic. Then again, there is now a collector specific name in the enum. Maybe the enum values should be made collector-specific in some way? Currently they already are (e.g. `_claim_finalizable` is only used in ZGC) as G1 does not need the values except for (multiple) `_claim_strong`.
> * I moved the CLD mark verification for the mark phase from `prepare_collection` to the constructor of `G1FullGCMarker`; I think this place is more fitting as directly above there is the use in the `CLDToOopClosure`. Also this pattern aligns with the use in the `G1FullGCAdjustTask`.
>
> Testing: tier1-5
>
> Thanks,
> Thomas
Looks good.
One comment below that you can decide if you address or not.
src/hotspot/share/classfile/classLoaderData.hpp line 210:
> 208: _claim_other = 4,
> 209: _claim_strong_g1_fullgc_mark = 8,
> 210: _claim_strong_g1_fullgc_adjust = 16
I do agree that the naming is not optimal and I wonder if using more generic names would be better for now. Like just `_claim_mark` and `_claim_adjust` and look at making them GC-specific should be a different change that addresses all GC specific values.
I also think `_claim_other` could continue to be last.
-------------
Marked as reviewed by sjohanss (Reviewer).
PR: https://git.openjdk.org/jdk/pull/10989
More information about the hotspot-dev
mailing list