RFR: 8376115: G1: Convert G1CMRootRegions to use Atomic<T>
Kim Barrett
kbarrett at openjdk.org
Fri Jan 23 19:03:06 UTC 2026
On Thu, 22 Jan 2026 15:00:17 GMT, Thomas Schatzl <tschatzl at openjdk.org> wrote:
> Hi all,
>
> please review this change to convert `G1CMRootRegions` to use `Atomic<T>`.
>
> Testing: gha
>
> Thanks,
> Thomas
A couple of optional small changes, otherwise looks good.
src/hotspot/share/gc/g1/g1ConcurrentMark.cpp line 419:
> 417:
> 418: size_t claimed_index = _claimed_root_regions.fetch_then_add(1u);
> 419: if (claimed_index < num_root_regions()) {
[pre-existing] Consider putting `num_root_regions()` in a variable for use both here and a
few lines above. The compiler can't CSE them, because of volatile/atomic accesses.
src/hotspot/share/gc/g1/g1ConcurrentMark.cpp line 430:
> 428:
> 429: bool G1CMRootMemRegions::contains(const MemRegion mr) const {
> 430: for (uint i = 0; i < num_root_regions(); i++) {
[pre-existing] Consider hoisting the loop limit out of the loop. Compiler can't do that here,
because of volatile/atomic accesses.
-------------
Marked as reviewed by kbarrett (Reviewer).
PR Review: https://git.openjdk.org/jdk/pull/29363#pullrequestreview-3699103240
PR Review Comment: https://git.openjdk.org/jdk/pull/29363#discussion_r2722457622
PR Review Comment: https://git.openjdk.org/jdk/pull/29363#discussion_r2722450369
More information about the hotspot-gc-dev
mailing list