RFR: 8276824: refactor Thread::is_JavaThread_protected [v2]
Daniel D.Daugherty
dcubed at openjdk.java.net
Fri Nov 12 19:10:46 UTC 2021
On Wed, 10 Nov 2021 02:54:07 GMT, David Holmes <dholmes at openjdk.org> wrote:
>> Daniel D. Daugherty has updated the pull request incrementally with one additional commit since the last revision:
>>
>> clarify the target thread parameter.
>
> Hi Dan,
>
> I'm okay with the changes as-is but also would not object to passing the current thread to avoid manifesting it - we do that a lot in other parts of the VM.
>
> One (multi-place) nit below.
>
> Thanks,
> David
@dholmes-ora - Thanks for re-review! I think we're good to go on this cleanup.
> src/hotspot/share/runtime/thread.cpp line 1764:
>
>> 1762: //
>> 1763: bool JavaThread::java_suspend() {
>> 1764: guarantee(Thread::is_JavaThread_protected_by_TLH(/* target */ this),
>
> Nit: there's no need to comment a single argument.
I added the comment due to this comment from @coleenp:
At the callsites below, it would make it obvious that "this" isn't meant to be the current thread (and calling Thread::current() inside a (tbd) assert would make it clear which thread is which.
I read @coleenp's comment as meaning that the uncommented `this` could be made more
obvious so I'm going to leave those two `/* target */` comments for clarity.
-------------
PR: https://git.openjdk.java.net/jdk/pull/6315
More information about the hotspot-runtime-dev
mailing list