RFR: 8364819: Post-integration cleanups for JDK-8359820 [v2]
David Holmes
dholmes at openjdk.org
Fri Aug 8 12:24:10 UTC 2025
On Fri, 8 Aug 2025 08:37:13 GMT, Aleksey Shipilev <shade at openjdk.org> wrote:
>>> acquire/release does not achieve that it only says what happens if you see the new value of field
>>
>> Note I am describing the semantic model for acquire/release in a general sense - as we describe in orderAccess.hpp. An actual implementation may ensure memory operations actually complete before the `release` in a similar way to a `fence` (e.g. X86 `mfence`).
>>
>> A "fence" may only ensure ordering in its abstract description but that suffices, as the store to the field must happen before any of the stores related to raising the signal, which must happen before the signal can actually be delivered, which happens before we load the field. Hence by transitivity we are guaranteed to see the fields written value, by using the fence.
>
> 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 with 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.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/26656#discussion_r2262830006
More information about the hotspot-dev
mailing list