RFR: 8359437: Make users and test suite not able to set LockingMode flag

Anton Artemov duke at openjdk.org
Mon Jun 23 14:45:47 UTC 2025


On Wed, 18 Jun 2025 07:37:31 GMT, David Holmes <dholmes at openjdk.org> wrote:

>> This PR contains changes for the 1st phase of the `LockingMode` flag obsoletion. 
>> 
>> The work is done by @fbredber, I have taken it over and am finishing it while he's on vacation. 
>> 
>> In the 1st phase one keeps the `LockingMode` variable in all places, but makes it non-settable from the command line. All the C1 and C2 code related to legacy locking will still be in place (but as dead code) and removed later (phase 2).
>> 
>> Lightweight locking is the default locking from now on.
>> 
>> Tested in tiers 1 - 7.
>
> src/hotspot/share/runtime/arguments.cpp line 1839:
> 
>> 1837: #ifndef _LP64
>> 1838:   if (LockingMode == LM_LEGACY) {
>> 1839:     LockingMode = LM_LIGHTWEIGHT;
> 
> If we have prevented the locking mode from being set then surely we can never encounter this case?

Looks like yes, this whole check then can be removed. Addressed in the latest commit.

> src/hotspot/share/utilities/globalDefinitions.cpp line 59:
> 
>> 57: uint64_t OopEncodingHeapMax = 0;
>> 58: 
>> 59: int LockingMode = LM_LIGHTWEIGHT;
> 
> const ?

This can be done provided that one removes assignment on line 3763 in arguments.cpp. That assignment looks redundant as LockingMode is always LM_LIGHTWEIGHT from now on.

> src/hotspot/share/utilities/globalDefinitions.hpp line 1012:
> 
>> 1010: };
>> 1011: 
>> 1012: extern int LockingMode;
> 
> const ?

This can be done provided that one removes assignment on line 3763 in arguments.cpp. That assignment looks redundant as LockingMode is always LM_LIGHTWEIGHT from now on.

> test/hotspot/jtreg/runtime/Monitor/StressWrapper_TestRecursiveLocking_36M.java line 36:
> 
>> 34:  *     -XX:+UnlockDiagnosticVMOptions -XX:+WhiteBoxAPI
>> 35:  *     -Xint
>> 36:  *     -XX:LockingMode=0
> 
> I was wondering why these LockingMode=0 test cases were not setting `VerifyHeavyMonitors` instead, but I'm assuming the intent now is that we will only test that mode when it is set externally by the user (or in our case a particular test task definition)?
> 
> I also realized we can only test heavy monitors in tests where we explicitly control the monitor creation places and hence can call the WB method to force inflation. That obviously reduces the test coverage for that mode quite significantly - but perhaps that will be handled if in the future we implicitly reenable forced inflation and do away with the WB usage.

My understanding is that VerifyHeavyMonitors requires LockingMode = 0, see line 1852 of arguments.cpp. So one has to set both at the same time, not one instead of another. Now locking mode is hardcoded to lightweight, and there is no way to use the incompatible `VerifyHeavyMonitors` option.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/25847#discussion_r2161284370
PR Review Comment: https://git.openjdk.org/jdk/pull/25847#discussion_r2161120254
PR Review Comment: https://git.openjdk.org/jdk/pull/25847#discussion_r2161119703
PR Review Comment: https://git.openjdk.org/jdk/pull/25847#discussion_r2161284143


More information about the hotspot-dev mailing list