RFR: 8265932: Move safepoint related fields from class Thread to JavaThread

[David Holmes]

On 27/04/2021 12:48 pm, patricio.chilano.mateo at oracle.com wrote:
> Hi David,
> 
> Thanks for looking at this.
> 
> On 4/26/21 9:48 PM, David Holmes wrote:
>> Hi Patricio,
>>
>> Changes looks good. I had to double-check a few JavaThread conversions 
>> actually appear in code that can only be executed by a JavaThread, but 
>> they seem okay.
>>
>> One comment below.
>>
>> Thanks,
>> David
>>
>> src/hotspot/share/runtime/mutex.cpp line 423:
>>
>>> 421:
>>> 422: // NSV implied with locking allow_vm_block or !safepoint_check 
>>> locks.
>>> 423: void Mutex::no_safepoint_verifier(Thread* thread, bool enable) {
>> Wouldn't it have been simpler to add the is_Java_thread() check in 
>> here rather than "inlining" this at the call-sites?
> Since the call can be replaced by just two lines I thought it was better 
> to inline it for readability purposes. I also didn't like the name 
> no_safepoint_verifier() too much since we are not really verifying 
> anything. The verification is done in check_possible_safepoint(). I now 
> see the same issue with NoSafepointVerifier (maybe it should be 
> NoSafepointMark). Anyways, I don't really have a strong opinion about it 
> so I can restore it if you think it's better with no_safepoint_verifier().

The review would have been simpler if this change had not been made, but 
now that it has ... I don't feel strongly enough to insist it be restored.

Thanks,
David
-----

> Thanks,
> Patricio
>> -------------
>>
>> Marked as reviewed by dholmes (Reviewer).
>>
>> PR: https://git.openjdk.java.net/jdk/pull/3695
>