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

David Holmes dholmes at openjdk.org
Mon Jun 23 14:45:46 UTC 2025


On Tue, 17 Jun 2025 08:39:49 GMT, Anton Artemov <duke 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.

I've taken an initial pass through. Initially I misunderstood the strategy with heavy monitors - see comments below.

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?

src/hotspot/share/utilities/globalDefinitions.cpp line 59:

> 57: uint64_t OopEncodingHeapMax = 0;
> 58: 
> 59: int LockingMode = LM_LIGHTWEIGHT;

const ?

src/hotspot/share/utilities/globalDefinitions.hpp line 1012:

> 1010: };
> 1011: 
> 1012: extern int LockingMode;

const ?

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.

test/hotspot/jtreg/runtime/vthread/JNIMonitor/JNIMonitor.java line 1:

> 1: /*

This seems to remove significant test coverage. can we not adapt the tests to not rely on logging warnings that will no longer be present?

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

PR Review: https://git.openjdk.org/jdk/pull/25847#pullrequestreview-2938076106
PR Review Comment: https://git.openjdk.org/jdk/pull/25847#discussion_r2153873165
PR Review Comment: https://git.openjdk.org/jdk/pull/25847#discussion_r2153924585
PR Review Comment: https://git.openjdk.org/jdk/pull/25847#discussion_r2153924946
PR Review Comment: https://git.openjdk.org/jdk/pull/25847#discussion_r2153884907
PR Review Comment: https://git.openjdk.org/jdk/pull/25847#discussion_r2153911036


More information about the hotspot-dev mailing list