RFR: 8366671: Refactor Thread::SpinAcquire and Thread::SpinRelease [v6]
Anton Artemov
aartemov at openjdk.org
Wed Nov 19 14:10:12 UTC 2025
On Tue, 18 Nov 2025 00:21:28 GMT, Kim Barrett <kbarrett at openjdk.org> wrote:
>> 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.
I agree that that this single critical section is an overkill and it does have only once use-case. And the implementation could be better. The agreement is that we don't need it. I have removed it.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/28264#discussion_r2542155560
More information about the hotspot-dev
mailing list