RFR: 8276824: refactor Thread::is_JavaThread_protected

[Coleen Phillimore]

On Tue, 9 Nov 2021 15:07:58 GMT, Daniel D. Daugherty <dcubed 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.

Thank you for this refactoring.  A small suggestion, we'll see if it's popular or not.

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.

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

Marked as reviewed by coleenp (Reviewer).

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