Removing ldar acquire from safepoint_poll method

Thu May 1 18:31:04 UTC 2025

Hi Samuel,

I’ll try to answer your questions below.

On 5/1/25 5:43 AM, Samuel Chee wrote:
> Hello,
>
> I was looking at whether it would be valid to remove the acquire from 
> safepoint_poll completely from here which emits an ldar instead of a 
> ldr for memory ordering. [1] As currently, there is only a single 
> instance of this argument being used, in the panama downstream ffi. [2]
> And similar changes have been made to remove the acquire argument from 
> other native methods [3] with the reasoning being that threads now 
> disarm their own poll value and hence an ldar is no longer needed. 
> Hence the same reasoning should apply when removing the acquire from 
> the safepoint_poll used in the panama ffi and all other uses of 
> safepoint_poll (especially since panama ffi is the only one which uses 
> acquire). If it applies, this means the acquire argument functionality 
> should be able to be removed completely from the safepoint_poll method.
>
> This change seemed to have been allowed when concurrent thread-stack 
> processing and watermarking was introduced and the 
> SafepointMechanism::update_poll_values method was updated such that 
> the poll value could not be armed when leaving this method. [4] [5] As 
> previously this may have not been the case as it could exit this 
> function with an armed poll value. [6]
> Given the complexity of the code - particularly around safepoints and 
> thread-handshake mechanisms, which have evolved significantly over 
> time - I’d like to clarify a few aspects to ensure this change is correct.
> My questions are:
>
> 1. What specifically allowed this change to be made to the other JNIs? 
> Why is the ldar acquire no longer needed?
As you found out from [1], the race is no longer present after 
JavaThreads disarm themselves. The race is well described by Erik in the 
links posted on that initial email.

> 2. Is there a reason why this was not done to the panama FFI 
> safepoint_poll when the acquire arguments were removed from the other 
> JNIs in [3]?
Seems the patch just meant to address the overhead on the JNI path, but 
this applies to the Panama one as well so we should be able to also use 
ldr. Let me know if you need help creating the JBS issue.

> 3. Why is a load_acquire required here [7] for before the handshake 
> operation or global state but not from some of the existing 
> jit-emitted code?
Good question. Once we see an armed poll we want to avoid memory 
operations to float up, otherwise we might for instance read stale 
values instead of the ones written by the arming thread before arming 
the poll. In particular, for handshakes we want to synchronize with the 
store release when arming the poll after the operation is added to the 
queue. For safepoints, the arming operation itself is done without extra 
fences but we previously executed storestore fences to order with some 
previous stores. I see SafepointMechanism::process() has a loadload 
fence and at least one early load acquire for the thread state so 
perhaps the acquire you mention when reading the poll word is not 
strictly needed. Erik and Robbin might have better insights.

> 4. Why did the introduction of watermarking introduce both a poll word 
> and poll page? Why is both needed?
Before 8253180 there were already two ways in which JavaThreads poll for 
safepoints/handshakes: testing the poll_bit of its polling_page or just 
trying to load from it (good page or bad page). Compiled methods use the 
latter method. I vaguely recall Erik trying to unify this and always 
test the poll_bit but I think there were some performance concerns. In 
any case, one field was enough. With 8253180, we now have to also check 
if we need to process frames when returning from a method. This requires 
a comparison of sp/fp with some previously saved reference, so we need 
an extra field.

Thanks,
Patricio

[1] 
https://mail.openjdk.org/pipermail/hotspot-compiler-dev/2024-July/078698.html
> 5. And any more additional context on how safepointing and poll values 
> work.
>
> Any clarification on the matter would be much appreciated!
> Samuel Chee
>
>
> [1] 
> _https://github.com/openjdk/jdk/blob/4c695fa8a459adcdb8cdfe9e90783007c65fb90e/src/hotspot/cpu/aarch64/macroAssembler_aarch64.cpp#L556 
> <https://github.com/openjdk/jdk/blob/4c695fa8a459adcdb8cdfe9e90783007c65fb90e/src/hotspot/cpu/aarch64/macroAssembler_aarch64.cpp#L556>_
> [2] 
> _https://github.com/openjdk/jdk/blob/482538b100856afe2252395d47e576e6c6d885ce/src/hotspot/cpu/aarch64/downcallLinker_aarch64.cpp#L292 
> <https://github.com/openjdk/jdk/blob/482538b100856afe2252395d47e576e6c6d885ce/src/hotspot/cpu/aarch64/downcallLinker_aarch64.cpp#L292>_
> [3] _https://bugs.openjdk.org/browse/JDK-8337657 
> <https://bugs.openjdk.org/browse/JDK-8337657>_
> [4] 
> _https://github.com/openjdk/jdk/pull/296/files#diff-2a18ec5d55c3c0c18669d55ff48fb455b633d152132915caf65766e41a5a58ab 
> <https://github.com/openjdk/jdk/pull/296/files#diff-2a18ec5d55c3c0c18669d55ff48fb455b633d152132915caf65766e41a5a58ab>_
> [5] _https://openjdk.org/jeps/376 <https://openjdk.org/jeps/376>_
> [6] 
> _https://github.com/openjdk/jdk/blob/a2f651904d1a1fdb14428be5485709b8ff252331/src/hotspot/share/runtime/safepointMechanism.cpp#L86 
> <https://github.com/openjdk/jdk/blob/a2f651904d1a1fdb14428be5485709b8ff252331/src/hotspot/share/runtime/safepointMechanism.cpp#L86>_,
> [7] 
> _https://github.com/openjdk/jdk/blob/765cef45465806e53f11fa7d92b9c184899b0932/src/hotspot/share/runtime/safepointMechanism.inline.hpp#L48 
> <https://github.com/openjdk/jdk/blob/765cef45465806e53f11fa7d92b9c184899b0932/src/hotspot/share/runtime/safepointMechanism.inline.hpp#L48>_
>
> IMPORTANT NOTICE: The contents of this email and any attachments are 
> confidential and may also be privileged. If you are not the intended 
> recipient, please notify the sender immediately and do not disclose 
> the contents to any other person, use it for any purpose, or store or 
> copy the information in any medium. Thank you. 
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/hotspot-runtime-dev/attachments/20250501/07df8971/attachment-0001.htm>