RFR: 8276824: refactor Thread::is_JavaThread_protected

[Daniel D.Daugherty]

On Tue, 9 Nov 2021 15:29:11 GMT, Coleen Phillimore <coleenp at openjdk.org> wrote:

>> Refactor `Thread::is_JavaThread_protected(const JavaThread* p, bool checkTLHOnly)`
>> into: `bool Thread::is_JavaThread_protected(const JavaThread* p)`
>> and: `Thread::is_JavaThread_protected_by_TLH(const JavaThread* p)`.
>> Also change callers that passed `checkTLHOnly == true` into calls to
>> `Thread::is_JavaThread_protected_by_TLH(const JavaThread* p)`.
>> 
>> This refactoring is being tested with Mach5 Tier[1-3] (in process).
>> 
>> In the "Files changed" view, enable "Settings -> Hide whitespace" for an easier review.
>
> src/hotspot/share/runtime/thread.cpp line 490:
> 
>> 488: // with the calling Thread?
>> 489: //
>> 490: bool Thread::is_JavaThread_protected_by_TLH(const JavaThread* p) {
> 
> I think you could pass JavaThread* current as the first parameter here.  The naming will help to keep it straight and make this an assert(current == JavaThread::current() if this is deemed to be expensive.
> 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.
> 
> With this handshake code I continuously am trying to figure out which thread is current vs. target.

I'm not fond of passing the current thread in as a parameter because that could
make folks think that these functions can be called on any `Thread*`. They must
be called in the context of the current thread which is also why they are static.
I would like to rename the `p` parameter to `target` and I could add `/*target*/`
to the call sites to make things more clear.

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

PR: https://git.openjdk.java.net/jdk/pull/6315