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

[Kim Barrett]

On Thu, 16 Oct 2025 14:45:45 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 incrementally with one additional commit since the last revision:
> 
>   default construct translated atomic without SFINAE

After a lot of Oracle-internal discussion, I'm proposing the `relaxed_store`
function in this PR be changed to `store_relaxed`. I'm going to hold off on
making that change for a bit, to give folks not in on that internal-to-Oracle
discussion an opportunity to weigh in. (I noticed that @theRealAph  liked the
suggestion of "unordered" in an earlier comment.)

Summarizing that discussion, here are goals and rationale for this change:

1. Have an explicit qualifier describing the behavior of the store. Some folks
have a strong preference for being explicit. Some folks have a preference for
the shorter `store` name (and `load` instead of `load_relaxed`), consistent
with `AtomicAccess`. It was agreed that the `AtomicAccess` names should be
updated to match, once the conversion to using `Atomic<T>` has been done.

2. Use the term "relaxed" for that qualifier. C, C++, and Rust use that
terminology. HotSpot uses `memory_order_relaxed` to be consistent with C++.
Java uses "opaque", but some felt that wasn't suggestive of any particular
behavior. Various other options (including "unordered" and "unfenced") were
considered, but none were sufficiently popular to overcome existing precedent.

3. Avoid visual ambiguity with `release_store`. This is the issue that started
the discussion.

There was some discussion of the "inconsistency" of `release_store` vs
`store_relaxed`, with the qualifier being in different positions. It was
suggested that, for naming consistency, all load/store qualifiers should
follow the operation. A noted benefit was that IDE name completion works well
in that case. But there was strong strong preference by some for
`release_store`, as it suggests the store is ordered with respect to preceding
operations. (And `load_aquire` orders the load with respect to following
operations.) The "relaxed" variants don't provide any ordering, so the
placement of the qualifier is unimportant in that regard. And "consistent"
placement leads to the visual ambiguity issue. The existence of
`release_store_fence` also makes any consistency argument look pretty weak.

In the dicussion about using "unordered" it was noted that our Access API uses
`MO_RELAXED` for atomic accesses without barriers, and `MO_UNORDERED` for
"plain" accesses. It's not obvious from the names how those differ, and
perhaps the name `MO_UNORDERED` should be reconsidered. But that's a separate
issue.

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

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