RFR: 8367013: Add Atomic<T> to package/replace idiom of volatile var plus AtomicAccess:: operations [v3]

[John R Rose]

On Wed, 15 Oct 2025 02:37:55 GMT, Kim Barrett <kbarrett at openjdk.org> wrote:

>> Please review this change that adds the type Atomic<T>, to use as the type
>> of a variable that is accessed (including writes) concurrently by multiple
>> threads. This is intended to replace (most) uses of the current HotSpot idiom
>> of declaring a variable volatile and accessing that variable using functions
>> from the AtomicAccess class.
>> https://github.com/openjdk/jdk/blame/528f93f8cb9f1fb9c19f31ab80c8a546f47beed2/doc/hotspot-style.md#L138-L147
>> 
>> This change replaces https://github.com/openjdk/jdk/pull/27462. Differences are
>> 
>> * Substantially restructured `Atomic<T>`, to be IDE friendly. It's
>> operationally the same, with the same API, hence uses and gtests didn't need
>> to change in that respect. Thanks to @stefank for raising this issue, and for
>> some suggestions toward improvements.
>> 
>> * Changed how fetch_then_set for atomic translated types is handled, to avoid
>> having the function there at all if it isn't usable, rather than just removing
>> it via SFINAE, leaving an empty overload set.
>> 
>> * Added more gtests.
>> 
>> Testing: mach5 tier1-6, GHA sanity tests
>
> Kim Barrett has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains 13 additional commits since the last revision:
> 
>  - Merge branch 'master' into atomic-template-tag-select
>  - Merge branch 'master' into atomic-template-tag-select
>  - add reference to gcc bug we're working around
>  - fix typo in comment
>  - update copyrights
>  - update FreeListAllocator
>  - update nonblockingQueue
>  - update LockFreeStack
>  - convert StringDedupTable
>  - convert SingleWriterSynchronizer
>  - ... and 3 more: https://git.openjdk.org/jdk/compare/1ac20fc3...67616416

My overall reaction is:  Yes, please, thank you Kim.  (I might have been one of the people who asked for this.)  Since we are using C++, of course we should tie together special data with special operations (in a class with non-static members), rather than relying on the mysteries of "volatile" hooking into an all-static API.

I'm glad we seem to be steering towards names and notations which are easier to learn, and also lead to code which is easier to read and reason about.  We have a code base which will live the better part of a century, which means code will be read many more times than written.

(And we shouldn't just use C++ atomics wholesale, because Java and HotSpot are extremely opinionated about concurrent memory access, down to the hardware level.  We need to set our own defaults and notations to dispel the bugs that are specific to our platform.)

So, I have run into "relaxed" and "opaque" many times in various related APIs.  (I remember RMO from SPARC days.)
I have been found both words hard to relate to the problems at hand, although I understand how they came about.
The arguments above against "relaxed" (at least two of them) seem to me worth considering.

I like Kim's suggestion of "unordered", meaning "no fences on this one".  ("Unfenced" also occurred to me FWIW.)
The bare names "load" and "store" are (by consensus) not explicit enough, and so we can't just drop the "fence modifier" from the function name.  Rather, we need a term which says in effect, "you were maybe expecting RELEASE-STORE, but there's no RELEASE, so it's just a a STORE; and that's why we call it UNORDERED-STORE".  (Or opaque or relaxed or unfenced or regular or whatever.)

Just my $0.02 on that naming point.  I will certainly defer to people who are working full time on the code base.

-------------

PR Comment: https://git.openjdk.org/jdk/pull/27539#issuecomment-3408701008