RFR: 8364819: Post-integration cleanups for JDK-8359820 [v2]

[Aleksey Shipilev]

On Fri, 8 Aug 2025 12:21:33 GMT, David Holmes <dholmes at openjdk.org> wrote:

>> As I said before, getting too deep into theory and how this case might be special is counter-productive here.
>> 
>> Honestly, I do _not_ understand the aversion to the idea that a field that is accessed by several threads should be nominally wrapped with `Atomic`. (Also note that some fields in `VMError` already do this.) Whatever the current situation is, that might allow for a low-level naked fence, the situation can change. I strongly believe we should be erring on the side of caution and future-proofness, unless there are other concerns on the table, like performance. There is no other concerns here, AFAICS. If you want a `fence` after the store for whatever reason, that's fine, there is `Atomic::release_store_fence` that gives it.
>
> It is not the "atomic" that is the issue it is the inappropriate and unnecessary use of acquire/release semantics. When people look at this code later they will wonder what it is that we are trying to coordinate - when the answer is "nothing". That just causes confusion. Synchronization and concurrency is hard enough without obfuscating things by sprinkling the wrong kind of synchronization constructs around "just to be safe". That isn't future-proofing things. If in the future we have different synchronization requirements then those requirements need to be understood and the correct code inserted. Maybe in the future we will need a mutex - who knows.

You say "inappropriate and unnecessary", I say "conservative". I think this is an opinion stalemate, so I'll just recluse myself from this conversation, and let someone else to break this tie.

There is no confusion to me: we pass something between the threads without clear additional synchronization in sight (i.e. mutexes) => we do `Atomic`-s. Deciding on the memory ordering for Atomics is: unless we see the need for performance, we default to conservative (acqrel and minimum for pointers, seqcst if we are are paranoid and cannot guarantee it is one-sided transfer) ordering, to avoid future accidents. 

If anything, a naked `fence()` raises much more questions for me in comparison with `Atomic` accesses. Mostly because fences are significantly more low-level than Atomics. When I look at this code from the perspective of bystander, these are the questions that pop into my mind: Why it is only the fence on the write side? Shouldn't there be a fence on the reader side somewhere then? Maybe we are optimizing performance? Are we relying on some other (partial) synchronization? Are we stacking with some other fence? Are there arch-specific details about what fences actually do? Etc.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/26656#discussion_r2262992787