RFR: 8262910: Cleanup THREAD/TRAPS/naming and typing issues in ObjectMonitor and related code

[Coleen Phillimore]

On 3/3/21 5:56 PM, David Holmes wrote:
> On 4/03/2021 2:28 am, Coleen Phillimore wrote:
>> I think this change looks good.  I made some general comments on 
>> specific lines.
>
> Thanks.
>
>> src/hotspot/share/runtime/synchronizer.hpp line 92:
>>
>>> 90:   // This is the "slow path" version of monitor enter and exit.
>>> 91:   static void enter(Handle obj, BasicLock* lock, JavaThread* 
>>> current);
>>> 92:   static void exit(oop obj, BasicLock* lock, TRAPS);
>>
>> So are you suggesting that we should have the convention that if a 
>> function takes a Thread/JavaThread argument as the last argument, to 
>> use "current" rather than "THREAD", since the latter implies it is 
>> supposed to be used to pass an argument for the parameter to TRAPS?  
>> I think this is good.  It manages some confusion about trailing 
>> THREAD arguments in some callers. Specializing further to JavaThread 
>> from Thread is also a good change.
>
> Yes. As discussed with Ioi, ideally if you see a call:
>
> foo(..., THREAD);
>
> then you should be able to say "Aha! foo is a TRAPS function that can 
> propagate an exception, but the calling code needs to do some special 
> handling so isn't using CHECK or CATCH". We are a long way from that 
> due to incidental uses of THREAD for convenience in things like 
> ResourceMark construction within TRAPS functions.

Yes, but please don't change ResourceMark rm(THREAD) to something else.  
It's not worth the churn and nobody is confused by it.
Coleen

>
> Thanks,
> David
> -----
>
>> src/hotspot/share/runtime/synchronizer.hpp line 146:
>>
>>> 144:
>>> 145:   // Deflate idle monitors:
>>> 146:   static void chk_for_block_req(JavaThread* current, const 
>>> char* op_name,
>>
>> I like "current" better than "self" as a convention.
>>
>> -------------
>>
>> Marked as reviewed by coleenp (Reviewer).
>>
>> PR: https://git.openjdk.java.net/jdk/pull/2802
>>