RFR: 8256425: Obsolete Biased Locking in JDK 18 [v2]

[Patricio Chilano Mateo]

On Thu, 17 Jun 2021 23:48:57 GMT, David Holmes <dholmes at openjdk.org> wrote:

>> Patricio Chilano Mateo has updated the pull request incrementally with two additional commits since the last revision:
>> 
>>  - remove test Test8062950.java + fix commments
>>  - fix comment in vm_version_ppc.cpp
>
> src/hotspot/share/code/nmethod.hpp line 281:
> 
>> 279:   nmethod(Method* method,
>> 280:           CompilerType type,
>> 281:           int nmethod_size,
> 
> Not obvious that this entire comment is no longer relevant. The basic description of the use of the offsets seems applicable even if not actually needed for revoking the bias.

Fixed. I restored most of the comment but remove the biased locking references.

> src/jdk.internal.vm.ci/share/classes/jdk.vm.ci.hotspot/src/jdk/vm/ci/hotspot/HotSpotVMConfig.java line 156:
> 
>> 154:     long prototypeMarkWord() {
>> 155:         return markWordNoHashInPlace | markWordNoLockInPlace;
>> 156:     }
> 
> It is not immediately obvious that this is correct for all object types. Does this match what the initial_mark() is now?

Yes, with biased locking gone there is now only one prototype for the markword. This should match markWord::prototype().

> test/hotspot/jtreg/compiler/c2/Test8062950.java line 2:
> 
>> 1: /*
>> 2:  * Copyright (c) 2018, 2021, Oracle and/or its affiliates. All rights reserved.
> 
> I'd argue this test serves no purpose now.

Right, removed.

> test/hotspot/jtreg/runtime/handshake/HandshakeDirectTest.java line 65:
> 
>> 63:                 // Inflate locks[handshakee] if possible
>> 64:                 System.identityHashCode(locks[handshakee]);
>> 65:                 walked = wb.handshakeReadMonitors(workingThreads[handshakee]);
> 
> It is not at all obvious that this revised test and the new WB routine actually test what was previously being tested. Do we actually need to involve monitors here or is that just something that has been picked to examine while in a handshake?

So the purpose of this test was to exercise direct handshakes between threads. Back then I used biased locking because it was the only one that was using them (and I didn't thought about using whitebox). Since the test has proven to be good to uncover some bugs I tried to keep the same elements. Walking the stack looking for monitors is one. Then I added System.identityHashCode() to force transitions from stack-locked to inflated whenever possible so that we also change the state of the lock as we did with biased locking (from biased to stack-locked).

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

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