8367014: Rename class Atomic to AtomicAccess #27135

kimbarrett · 2025-09-08T06:26:03Z

Please review this change that renames the all-static class Atomic to
AtomicAccess. The reason for this name change is to allow the introduction
of the new type Atomic<T> (JDK-8367013).

The PR has several commits, according to the specific category of change being
made. It may be easier to review the PR by studying these individual commits.

Although the file "atomic.hpp" is being renamed to "atomicAccess.hpp", I chose
to not rename the various "atomic_." and "atomic__." files.

There are a number of comments containing the word "Atomic" that I didn't
change. They are generically about atomic operations, and will just as well
serve as referring to the future Atomic<T>.

Testing: mach5 tier1, GHA sanity tests.
This is one of those changes where successful builds indicate the change is good.

Progress

Change must be properly reviewed (1 review required, with at least 1 Reviewer)
Change must not contain extraneous whitespace
Commit message must refer to an issue

Issue

JDK-8367014: Rename class Atomic to AtomicAccess (Enhancement - P4)

Reviewers

David Holmes (@dholmes-ora - Reviewer)
Andrew Haley (@theRealAph - Reviewer) Review applies to 4ad4f820
Stefan Karlsson (@stefank - Reviewer) Review applies to 4ad4f820

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/27135/head:pull/27135
$ git checkout pull/27135

Update a local copy of the PR:
$ git checkout pull/27135
$ git pull https://git.openjdk.org/jdk.git pull/27135/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 27135

View PR using the GUI difftool:
$ git pr show -t 27135

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/27135.diff

Using Webrev

Link to Webrev Comment

bridgekeeper · 2025-09-08T06:26:41Z

👋 Welcome back kbarrett! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

openjdk · 2025-09-08T06:27:36Z

@kimbarrett This change now passes all automated pre-integration checks.

ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details.

After integration, the commit message for the final commit will be:

8367014: Rename class Atomic to AtomicAccess

Reviewed-by: dholmes, aph, stefank

You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed.

At the time when this comment was updated there had been 3 new commits pushed to the master branch:

2826d17: 8367243: Format issues with dist dump debug output in PhaseGVN::dead_loop_check
4cc75be: 8366702: C2 SuperWord: refactor VTransform vector nodes
eb9e045: 8361530: Test javax/swing/GraphicsConfigNotifier/StalePreferredSize.java timed out

Please see this link for an up-to-date comparison between the source branch of this pull request and the master branch.
As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details.

➡️ To integrate this PR with the above commit message to the master branch, type /integrate in a new comment.

openjdk · 2025-09-08T06:28:13Z

@kimbarrett The following labels will be automatically applied to this pull request:

graal
hotspot
serviceability
shenandoah

When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing lists. If you would like to change these labels, use the /label pull request command.

mlbridge · 2025-09-08T06:32:07Z

Webrevs

02: Full (00ecd55c)
01: Full (11007c45)
00: Full (4ad4f820)

dholmes-ora

LGTM! Thanks for taking this on!

theRealAph

AtomicAccess is a bit wordy, and this change is going to mess up diffs for backports terribly, but I can't think of a better way to do it. Thanks.

kimbarrett · 2025-09-08T09:44:26Z

AtomicAccess is a bit wordy, and this change is going to mess up diffs for backports terribly, but I can't think of a better way to do it. Thanks.

There was a bunch of internal to Oracle bike shedding over the names already. But I'm open to more if someone
thinks they have a better idea. Note that once we're all done with switching to Atomic<T> where appropriate,
I don't expect very many direct uses of AtomicAccess to remain (though there will be some).

Diffs for backports are going to get messed up anyway, since most uses of AtomicAccess will eventually be
switched over to Atomic<T> style usage.

theRealAph · 2025-09-08T10:15:18Z

AtomicAccess is a bit wordy, and this change is going to mess up diffs for backports terribly, but I can't think of a better way to do it. Thanks.

There was a bunch of internal to Oracle bike shedding over the names already.

Sure, I imagine there was! It's a shame when decision making happens behind closed doors in a FOSS project, but public list bikeshedding would have been too much for this change.

Diffs for backports are going to get messed up anyway, since most uses of AtomicAccess will eventually be switched over to Atomic<T> style usage.

That's fair.

stefank

I've checked through the patch and it looks good. I found one file that lacked alignment adjustments.

Although the file "atomic.hpp" is being renamed to "atomicAccess.hpp", I chose
to not rename the various "atomic_." and "atomic__." files.

Could you motivate why you chose to not do that?

stefank · 2025-09-09T11:20:47Z

src/hotspot/os_cpu/bsd_x86/atomic_bsd_x86.hpp

 template<>
 template<typename D, typename I>
-inline D Atomic::PlatformAdd<4>::fetch_then_add(D volatile* dest, I add_value,
+inline D AtomicAccess::PlatformAdd<4>::fetch_then_add(D volatile* dest, I add_value,


This file has multiple alignment issues.

Oops, completely missed that file. Will fix.

openjdk · 2025-09-09T11:51:04Z

@kimbarrett this pull request can not be integrated into master due to one or more merge conflicts. To resolve these merge conflicts and update this pull request you can run the following commands in the local repository for your personal fork:

git checkout atomic-access
git fetch https://git.openjdk.org/jdk.git master
git merge FETCH_HEAD
# resolve conflicts and follow the instructions given by git merge
git commit -m "Merge master"
git push

kimbarrett · 2025-09-10T07:23:58Z

Although the file "atomic.hpp" is being renamed to "atomicAccess.hpp", I chose
to not rename the various "atomic_." and "atomic__." files.

Could you motivate why you chose to not do that?

I thought about it, and waffled back and forth. But I was trying to do as much
as possible of this change mechanically. Renaming a file involves multiple
steps that weren't all easily scriptable. (And I'd already messed up a part of
the renaming of atomic.hpp during patch development.) Also, this change is
going to be hard for backports as it is, and I think renamings might make that
worse. Renamings can also be annoying for archeology. But if you think it's
important...

kimbarrett · 2025-09-10T07:26:02Z

Updated for recent commits adding some new uses of Atomic:: and merge conflicts. Running GHA sanity tests.
mach5 tier1 already passed.

theRealAph · 2025-09-10T07:40:38Z

Also, this change is
going to be hard for backports as it is, and I think renamings might make that
worse. Renamings can also be annoying for archeology.

Speaking as an archaeologist and the lead of multiple backport projects, I agree with you, Kim.

stefank · 2025-09-10T08:53:47Z

Although the file "atomic.hpp" is being renamed to "atomicAccess.hpp", I chose
to not rename the various "atomic_." and "atomic__." files.

Could you motivate why you chose to not do that?

I thought about it, and waffled back and forth. But I was trying to do as much as possible of this change mechanically. Renaming a file involves multiple steps that weren't all easily scriptable. (And I'd already messed up a part of the renaming of atomic.hpp during patch development.) Also, this change is going to be hard for backports as it is, and I think renamings might make that worse. Renamings can also be annoying for archeology. But if you think it's important...

Sure, renames are annoying. I do think that it is bad to leave inconsistent names in a long-lived, evolving code base.

kimbarrett · 2025-09-11T04:14:20Z

src/hotspot/cpu/aarch64/gc/shared/barrierSetNMethod_aarch64.cpp

    return -4 * (4 + slow_path_size(nm));
  case NMethodPatchingType::conc_instruction_and_data_patch:
    return -4 * (10 + slow_path_size(nm));
  }


This and similar changes for arm and riscv came from a merged changeset.
I took that new version and updated it for Atomic:: => AtomicAccess:: renaming.

kimbarrett · 2025-09-11T04:15:13Z

src/hotspot/share/jfr/periodic/sampling/jfrCPUTimeThreadSampler.cpp

    return nullptr;
  }
  return jt;
 }


Another case of taking a merge update and then adjusting for Atomic:: => AtomicAccess::.

kimbarrett · 2025-09-11T04:15:44Z

src/hotspot/share/runtime/basicLock.inline.hpp


 #include "runtime/objectMonitor.inline.hpp"

-inline markWord BasicLock::displaced_header() const {


These functions were removed by a merge update.

kimbarrett · 2025-09-11T04:18:09Z

Needs re-review because of updates to deal with merge conflicts and to update newly merged code that needed
the renamings applied. @stefank @theRealAph @dholmes-ora

kimbarrett · 2025-09-11T04:19:24Z

Hm, now says there is a new merge conflict. Guess you folks should hold off on re-review.

dholmes-ora

Still good. Thanks

kimbarrett · 2025-09-12T06:32:58Z

Thanks for reviews @theRealAph , @stefank , and @dholmes-ora

kimbarrett · 2025-09-12T06:33:08Z

/integrate

openjdk · 2025-09-12T06:35:56Z

Going to push as commit 9e843f5.
Since your change was applied there have been 19 commits pushed to the master branch:

5abd184: 8367137: RISC-V: Detect Zicboz block size via hwprobe
e172e6a: 8352969: G1: Improve testability of optional collections
7603e96: 8366911: (fs) Remove support for normalizing file names to Unicode normalized form D (macOS)
... and 16 more: https://git.openjdk.org/jdk/compare/134c3ef41e774b483bcce32ce2fe0ef416017728...master

Your commit was automatically rebased without conflicts.

openjdk · 2025-09-12T06:36:06Z

@kimbarrett Pushed as commit 9e843f5.

💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.

kimbarrett added 8 commits September 6, 2025 18:30

rename atomic.hpp => atomicAccess.hpp

181c840

change includes of atomic.hpp

f605a00

change includes of atomic.hpp in gtests

5a45009

rename Atomic => AtomicAccess

4120d42

rename Atomic => AtomicAccess in gtests

0987653

fix indentation from rename

71829b5

misc cleanups

a9a3c72

update copyrights

4ad4f82

openjdk bot added graal graal-dev@openjdk.org serviceability serviceability-dev@openjdk.org hotspot hotspot-dev@openjdk.org shenandoah shenandoah-dev@openjdk.org rfr Pull request is ready for review labels Sep 8, 2025

dholmes-ora approved these changes Sep 8, 2025

View reviewed changes

openjdk bot added the ready Pull request is ready to be integrated label Sep 8, 2025

theRealAph approved these changes Sep 8, 2025

View reviewed changes

rename test_atomic.cpp

9c2c871

stefank approved these changes Sep 9, 2025

View reviewed changes

openjdk bot added merge-conflict Pull request has merge conflict with target branch and removed ready Pull request is ready to be integrated labels Sep 9, 2025

kimbarrett added 3 commits September 10, 2025 04:54

fix prefiously missed arg misalignments

f53265b

Merge branch 'master' into atomic-access

b6b65bf

rename recently added Atomic:: => AtomicAccess::

11007c4

openjdk bot removed the merge-conflict Pull request has merge conflict with target branch label Sep 10, 2025

kimbarrett commented Sep 11, 2025

View reviewed changes

Merge branch 'master' into atomic-access

00ecd55

dholmes-ora approved these changes Sep 11, 2025

View reviewed changes

openjdk bot added the ready Pull request is ready to be integrated label Sep 11, 2025

openjdk bot added the integrated Pull request has been integrated label Sep 12, 2025

openjdk bot closed this Sep 12, 2025

openjdk bot removed ready Pull request is ready to be integrated rfr Pull request is ready for review labels Sep 12, 2025

kimbarrett deleted the atomic-access branch September 12, 2025 07:36

Hamlin-Li mentioned this pull request Sep 12, 2025

8367501: RISC-V: build broken after JDK-8365926 #27251

Closed

3 tasks


		#include "runtime/objectMonitor.inline.hpp"

		inline markWord BasicLock::displaced_header() const {

8367014: Rename class Atomic to AtomicAccess #27135

8367014: Rename class Atomic to AtomicAccess #27135

Uh oh!

Conversation

kimbarrett commented Sep 8, 2025 • edited by openjdk bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Progress

Issue

Reviewers

Reviewing

Uh oh!

bridgekeeper bot commented Sep 8, 2025

Uh oh!

openjdk bot commented Sep 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

openjdk bot commented Sep 8, 2025

Uh oh!

mlbridge bot commented Sep 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Webrevs

Uh oh!

dholmes-ora left a comment

Choose a reason for hiding this comment

Uh oh!

theRealAph left a comment

Choose a reason for hiding this comment

Uh oh!

kimbarrett commented Sep 8, 2025

Uh oh!

theRealAph commented Sep 8, 2025

Uh oh!

stefank left a comment

Choose a reason for hiding this comment

Uh oh!

stefank Sep 9, 2025

Choose a reason for hiding this comment

Uh oh!

kimbarrett Sep 10, 2025

Choose a reason for hiding this comment

Uh oh!

openjdk bot commented Sep 9, 2025

Uh oh!

kimbarrett commented Sep 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kimbarrett commented Sep 10, 2025

Uh oh!

theRealAph commented Sep 10, 2025

Uh oh!

stefank commented Sep 10, 2025

Uh oh!

kimbarrett Sep 11, 2025

Choose a reason for hiding this comment

Uh oh!

kimbarrett Sep 11, 2025

Choose a reason for hiding this comment

Uh oh!

kimbarrett Sep 11, 2025

Choose a reason for hiding this comment

Uh oh!

kimbarrett commented Sep 11, 2025

Uh oh!

kimbarrett commented Sep 11, 2025

Uh oh!

dholmes-ora left a comment

Choose a reason for hiding this comment

Uh oh!

kimbarrett commented Sep 12, 2025

Uh oh!

kimbarrett commented Sep 12, 2025

Uh oh!

openjdk bot commented Sep 12, 2025

Uh oh!

openjdk bot commented Sep 12, 2025

Uh oh!

Reviewers

Assignees

Labels

Milestone

kimbarrett commented Sep 8, 2025 •

edited by openjdk bot

Loading

openjdk bot commented Sep 8, 2025 •

edited

Loading

mlbridge bot commented Sep 8, 2025 •

edited

Loading

kimbarrett commented Sep 10, 2025 •

edited

Loading