RFR: 8366038: Thread::SpinRelease should use Atomic::release_store

David Holmes dholmes at openjdk.org
Tue Sep 2 08:04:49 UTC 2025


On Mon, 1 Sep 2025 14:34:29 GMT, Anton Artemov <duke at openjdk.org> wrote:

> Hi, please consider the following changes:
> 
> `Thread::SpinRelease()` uses an `OrderAccess::fence()` to prevent loads and stores from within the critical section to float down out of the section. A full fence is expensive and it is an overkill on most platforms. Instead, one can achieve the same effect with `Atomic::release_store()`. 
> 
> Tested in tiers 1 - 3.

Looks good, but I think we can shorten the mega-commentary. Thanks

src/hotspot/share/runtime/thread.cpp line 614:

> 612:   // Conceptually we need a #loadstore|#storestore "release" MEMBAR before
> 613:   // the ST of 0 into the lock-word which releases the lock, so fence
> 614:   // more than covers this on all platforms.

Suggestion:

  // So we need a #loadstore|#storestore "release" memory barrier before
  // the ST of 0 into the lock-word which releases the lock.

src/hotspot/share/runtime/thread.cpp line 616:

> 614:   // more than covers this on all platforms.
> 615:   // However, a full fence is an overkill on most platforms,
> 616:   // the same effect can be achieved with Atomic::release_store().

Suggestion:

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

Changes requested by dholmes (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/27035#pullrequestreview-3175332019
PR Review Comment: https://git.openjdk.org/jdk/pull/27035#discussion_r2315252612
PR Review Comment: https://git.openjdk.org/jdk/pull/27035#discussion_r2315253345


More information about the hotspot-runtime-dev mailing list