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