RFR: 8366671: Refactor Thread::SpinAcquire and Thread::SpinRelease [v6]

[Kim Barrett]

On Mon, 17 Nov 2025 22:49:04 GMT, Kim Barrett <kbarrett at openjdk.org> wrote:

>> Anton Artemov has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   8366671: Removed redundant include.
>
> src/hotspot/share/utilities/spinCriticalSection.hpp line 56:
> 
>> 54: 
>> 55: template<class Lambda, class...Args>
>> 56: class SpinSingleSection {
> 
> Although I've made some suggestions for possibly improving SpinSingleSection,
> I'm not sure it's a good idea as a concept. It seems to be attempting to
> provide a conditional critical section, but is doing so in what seems to me to
> be a weird way.
> 
> As provided, it first conditionally executes a funarg under the
> lock, if it can acquire the lock. It then permits an external body (the scope
> of the section) to execute either under or not under the lock (depending on
> whether it was successfully acquired), with no way to know which state we're
> in.
> 
> I think an API more similar to `std::unique_lock` for SpinCriticalSection
> would be better. `std::unique_lock` provides a `owns_lock()` function and a
> constructor overload taking a `std::try_to_lock_t` value. This controls
> whether the locking should be conditional or not, and a way for the using code
> to detect success/failure to lock in the conditional case. This doesn't have
> to be in one class though. There could be two critical section classes, one
> unconditional and one conditional, with only the latter providing the
> success/failure info.

Or maybe just not bother with special help for the currently one(?) use-case for this, and instead
have that use-case directly use `try_acquire` with a local RAII object to ensure release in the
acquired case. Or a local bespoke helper class, or something along those lines.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/28264#discussion_r2535907269