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

[Chris Plummer]

On Fri, 18 Jun 2021 15:04:28 GMT, Patricio Chilano Mateo <pchilanomate at openjdk.org> wrote:

>> Hi all,
>> 
>> Please review the following patch which handles the removal of biased locking code. 
>> 
>> The third least significant bit of the markword is now always unused. I didn't try to give it back to the age field as it was prior to biased locking introduction since it will likely be taken away by other projects (probably Valhalla). 
>> 
>> Regarding c1 changes, the scratch register passed to LIRGenerator::monitor_enter() was only used by biased locking code except in ppc, so in all other platforms I removed the scratch parameter from C1_MacroAssembler::lock_object() (except in s390 where it wasn't defined already). 
>> We could probably just always use R0 as a temp register in lock_object() for ppc, since we were already using it as temp in biased_locking_enter(), and remove the scratch parameter from there too. Then we could remove the scratch field from LIR_OpLock. I haven't done that in this patch though.
>> 
>> For c2, type.hpp defined XorXNode, StoreXConditionalNode, LoadXNode and StoreXNode as needed by UseOptoBiasInlining. I see that LoadXNode and StoreXNode are also used by shenandoahSupport so I kept those two defines. I removed only the biased locking comments from the storeIConditional/storeLConditional implementations in .ad files since I don't know if they might be needed.
>> 
>> There are some tests that were only meaningful when run with biased locking enabled so I removed them.
>> 
>> Tested in mach5 tiers 1-7. I tested it builds also on ppc, s390 and arm32 but can't run any tests on those platforms so it would be good if somebody can do some sanity check on those ones.
>> 
>> Thanks,
>> Patricio
>
> 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

test/jdk/com/sun/jdi/EATests.java line 52:

> 50:  *                 -XX:+WhiteBoxAPI
> 51:  *                 -Xbatch
> 52:  *                 -XX:+DoEscapeAnalysis -XX:+EliminateAllocations -XX:-EliminateLocks -XX:+EliminateNestedLocks -XX:+UseBiasedLocking -XX:-UseOptoBiasInlining

I don't see this combination of flags in the new diff. I think the approach should be to remove the biased locking flags, and then remove any duplicate test runs that result from doing that.

test/jdk/com/sun/jdi/EATests.java line 235:

> 233:         // Relocking test cases
> 234:         new EARelockingSimpleTarget()                                                       .run();
> 235:         new EARelockingSimple_2Target()                                                     .run();

I know all the tests that were removed mention biased locking in the comments, but do they require biased locking to function properly? I'm just wondering if we might get better EA test coverage if they are left in place.

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

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