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

[Kim Barrett]

On Tue, 21 Oct 2025 13:27:27 GMT, Emanuel Peter <epeter at openjdk.org> wrote:

>> Kim Barrett has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   default construct translated atomic without SFINAE
>
> src/hotspot/share/runtime/atomic.hpp line 84:
> 
>> 82: // Default construction of an atomic integer or atomic byte initializes the
>> 83: // value to zero. Default construction of an atomic pointer initializes the
>> 84: // value to null.
> 
> Suggestion:
> 
> // value to nullptr.
> 
> Optional, and total nit-pick. Reason: we are in C++ and not Java.

We generally use "null" in comments, unless it's part of a code snippet.

> src/hotspot/share/runtime/atomic.hpp line 160:
> 
>> 158: //
>> 159: // * Rather than load/store operations with a memory order parameter,
>> 160: // Atomic<T> provides load_{relaxed,acquire}() and {relaxed,release}_store()
> 
> Optional, and totally nit-picky: consider writing them out, so we can grep easily.

Done. I'd missed that "relaxed_store" when renaming to "store_relaxed".

> test/hotspot/gtest/runtime/test_atomic.cpp line 33:
> 
>> 31: 
>> 32: // These tests of Atomic<T> only verify functionality.  They don't verify
>> 33: // atomicity.
> 
> Would that be desirable? Do we have tests, or an issue for it to do it in the future?

The comment is there to set expectations. Actually testing atomicity seems
like it would be really hard (maybe expensive and complex stress tests?), and
of little benefit. Failures could only arise if we have an obviously wrong
implementation (missing "lock" prefix, ll/sc, ordered load/store operations,
&etc as appropriate to the architecture), a broken compiler, or broken
hardware.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/27539#discussion_r2448658705
PR Review Comment: https://git.openjdk.org/jdk/pull/27539#discussion_r2448659543
PR Review Comment: https://git.openjdk.org/jdk/pull/27539#discussion_r2448656589