RFR: 8255678: Add Mutex::try_lock version without rank checks [v5]

[Patricio Chilano Mateo]

On Thu, 19 Nov 2020 07:50:51 GMT, David Holmes <dholmes at openjdk.org> wrote:

>> Patricio Chilano Mateo has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   early test check_can_be_skipped + extra try_lock test
>
> src/hotspot/share/runtime/mutex.cpp line 148:
> 
>> 146:   // Checking the owner hides the potential difference in recursive locking behaviour
>> 147:   // on some platforms.
>> 148:   if(owned_by_self(self)) {
> 
> Need space after if

Fixed.

> src/hotspot/share/runtime/mutex.cpp line 152:
> 
>> 150:   }
>> 151: 
>> 152:   if(do_rank_checks) {
> 
> need space after if

Fixed.

> src/hotspot/share/runtime/mutex.cpp line 310:
> 
>> 308:   Mutex(Rank, name, allow_vm_block, safepoint_check_required) {}
>> 309: 
>> 310: bool Mutex::owned_by_self(Thread* thread) const {
> 
> Side comment: I'm not sure this is the right way to go versus just checking _owner directly. If you go this style then we end up with the potential for owned_by_self(self); owned_by_self(); not_owned() /* compares _owner == NULL */ and a similar range of assertions - rather than just comparing _owner directly (and with owned_by_self() convenience function).

Right, I just changed it to check the owner directly.

> src/hotspot/share/runtime/mutex.hpp line 204:
> 
>> 202:   Thread* owner() const         { return _owner; }
>> 203:   void set_owner(Thread* owner) { set_owner_implementation(owner); }
>> 204:   bool owned_by_self(Thread* thread) const;
> 
> Does this need to be public? Arguably when passing in a thread this should just be owned_by(x) - unless you'd also like to assert that x == Thread::current() ?

I removed set_owner(Thread* owner).

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

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