RFR: 8364819: Post-integration cleanups for JDK-8359820 [v2]
David Holmes
dholmes at openjdk.org
Thu Aug 7 22:24:11 UTC 2025
On Thu, 7 Aug 2025 12:34:21 GMT, Aleksey Shipilev <shade at openjdk.org> wrote:
>>> that might miss updates as well.
>>
>> Might miss updates to what??? We never follow the pointer. The signalling thread doesn't doesn't do anything that the signallee has to look at, the signallee just gets the signal, checks if it was the timeout target and aborts.
>>
>> We use fence() for "single variable visibility reasons" in a number of places because it is the only thing that actually forces memory synchronization across all threads. acquire/release just says "if you see this then you must see that", it doesn't do anything to help you "see this" in the first place.
>
>> Might miss updates to what??? We never follow the pointer.
>
> Following the pointer is not relevant. We can miss updates to the thread pointer. Note that reader needs at least coherence to break out of potential loops like:
>
>
> while (get_handshake_thread() == nullptr); // burn
>
>
> Acquire gives that, among other things. Overlooking these properties is easy, and this is why we should be sticking to use atomics anywhere we transfer data between threads.
>
>> We use fence() for "single variable visibility reasons" in a number of places because it is the only thing that actually forces memory synchronization across all threads.
>
> Well, yes, this part always felt dubious to me. I am restraining myself from going into theoretical argument here, because it would take a lot of time, and would not be helpful for this work.
>
> Since no performance is on the table, we should just do the most obvious/safe thing. I believe it is a proper style to use `Atomic::load_acquire` and `Atomic::release_store_fence`, instead of one-sided `fence`.
Blindly applying acquire/release is not at all the obvious/safe thing to me. I do not understand your reference to `get_handshake_thread()` above - this is not about handshakes. Here we have a very simple situation:
- Thread 1: set field; signal target thread
- Target thread: receive signal; read field
All we need to "transfer" here is the value of "field". acquire/release does not achieve that it only says what happens _if_ you see the new value of field.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/26656#discussion_r2261569500
More information about the hotspot-dev
mailing list