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

[Kim Barrett]

On Wed, 22 Oct 2025 06:39:49 GMT, Axel Boldt-Christmas <aboldtch at openjdk.org> wrote:

> I played around with this a bit. I think the implementation and API looks good, but have a couple of questions and I found one issue with certain types which unexpectedly do not compile.
> 
> So in the following code snippet neither `Atomic<EnumClass16>` nor `Atomic<uint16_t>` can be compiled.

16bit values are not supported because we've never implemented support for
them in the AtomicAccess layer. Presumably that's because nobody has had a
use-case for them. Or at least not one compelling enough to convince someone
to do the implementation work (across all platforms). This is discussed in the
"notes" toward the end of the big block comment at the head of atomic.hpp.

> src/hotspot/share/runtime/atomic.hpp line 299:
> 
>> 297:                    atomic_memory_order order = memory_order_conservative) {
>> 298:     return AtomicAccess::xchg(this->value_ptr(), new_value);
>> 299:   }
> 
> I am not sure I understood the motivation behind the changing of the `xchg` name, nor why `cmpxchg` did not get a similar treatment. Could you provide some elaboration here?
> 
> Is there any plan to change the naming in the AtomicAccess layer as well?

The name "xchg" is pretty generic (it could just as easily be used for a
non-atomic exchange in some other class). The explicit `Atomic::` (now
`AtomicAccess::`) qualifier made for easy disambiguation. Similarly for the
others, like "inc" and "dec", and not providing short forms for "add" and
"sub". (And remember that we mostly eschew operator overloading, though I
think not always to our benefit. If not for that, such other classes probably
would be doing that rather than named operations for arithmetic.) I think
"cmpxchg" is pretty well understood to be atomic; I think if one tried to use
that name for a non-atomic operation in some other class one might well get
some push-back.

Personally, I also find the short names are harder to spot in the middle of
other code, and I liked that `AtomicAccess::foo` (formerly `Atomic::foo` was
easy to spot.

I think updating AtomicAccess to be consistent with what we decide for
Atomic<T> should be done, but only after Atomic<T> has been fully adopted, so
we're only dealing the the remaining resudue. This has already been discussed
with regard to explicit "relaxed" load/store qualifiers.

> test/hotspot/gtest/runtime/test_atomic.cpp line 604:
> 
>> 602: // Verify that explicit instantiation doesn't attempt to reference the
>> 603: // non-existent fetch_then_set of the atomic decayed type.
>> 604: template class AtomicImpl::Atomic<TranslatedAtomicByteObject>;
> 
> Preexisting: 
> Why do we not suport exchange on 8 and 16 byte sized types? 
> 
> Especially given that we have `AtomicAccess::XchgUsingCmpxchg` and do support `cmpxchg`.

Again, what's the use-case, and someone needs to do the work. That's all.
Just a mere matter of programming...

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

PR Comment: https://git.openjdk.org/jdk/pull/27539#issuecomment-3432577864
PR Review Comment: https://git.openjdk.org/jdk/pull/27539#discussion_r2452239941
PR Review Comment: https://git.openjdk.org/jdk/pull/27539#discussion_r2452245123