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

[David Holmes]

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.

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
>