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