On Tue, 23 Jul 2024 16:44:06 GMT, Coleen Phillimore <cole...@openjdk.org> wrote:
>> I wanted to avoid having to add `NoSafepointVerifier` implementation details
>> in the synchroniser code. I guess `ContinuationWrapper` already does this.
>>
>> Simply creating a `NoSafepointVerifier` when you expect no safepoint is more
>> obvious to me, shows the intent better.
>
> This looks strange to me also, but it's be better than changing the
> no_safepoint_count directly, since NSV handles when the current thread isn't
> a JavaThread, so you'd have to duplicate that in this VerifyThreadState code
> too.
>
> NoSafepointVerifier::NoSafepointVerifier() : _thread(Thread::current()) {
> if (_thread->is_Java_thread()) {
> JavaThread::cast(_thread)->inc_no_safepoint_count();
> }
> }
It was the call to `[inc/dec]_no_safepoint_count` I wanted to avoid. But I will
switch the conditionally created NSV to the `[inc/dec]_no_safepoint_count`
calls instead.
>> Yeah. The only effect is has is that you cannot reassign the variable. It
>> was the style taken from
>> [synchronizer.hpp](https://github.com/openjdk/jdk/blob/15997bc3dfe9dddf21f20fa189f97291824892de/src/hotspot/share/runtime/synchronizer.hpp)
>> where all `InflateCause` parameters are const.
>
> Do you get this for inflate_fast_locked_object also?
Yes. I'll just remove the const from all lightweightSynchronizer parameters.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/20067#discussion_r1713909267
PR Review Comment: https://git.openjdk.org/jdk/pull/20067#discussion_r1713909417
