On Wed, 18 Jun 2025 18:23:58 GMT, Coleen Phillimore <cole...@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.
>
> test/hotspot/jtreg/runtime/Monitor/TestRecursiveLocking.java line 125:
>
>> 123: public class TestRecursiveLocking {
>> 124: static final WhiteBox WB = WhiteBox.getWhiteBox();
>> 125: static final boolean flagHeavyMonitors =
>> WB.getBooleanVMFlag("VerifyHeavyMonitors");
>
> I think you should take out the VerifyHeavyMonitors cases. @fbredber
> originally had that flag turn on a the reintroduced UseHeavyMonitors option
> but the UseHeavyMonitors option doesn't actually do that with this change. I
> don't think this test will pass with -XX:+VerifyHeavyMonitors.
> If we reintroduce UseHeavyMonitors, save this diff and fix this test then.
> Right now it's not correct.
Removed in the latest commit.
> test/hotspot/jtreg/runtime/lockStack/TestLockStackCapacity.java line 42:
>
>> 40: public class TestLockStackCapacity {
>> 41: static final WhiteBox WB = WhiteBox.getWhiteBox();
>> 42: static final boolean flagHeavyMonitors =
>> WB.getBooleanVMFlag("VerifyHeavyMonitors");
>
> I think this should also not have cases for VerifyHeavyMonitors. We can add
> back tests if we want UseHeavyMonitors. As of now, removing the Legacy
> locking code will remove code that reaches the VerifyHeavyMonitors branches.
Removed in the latest commit.
> test/jtreg-ext/requires/VMProps.java line 424:
>
>> 422: * Note: Lightweight locking does not support RTM (for now).
>> 423: */
>> 424: protected String vmRTMCompiler() {
>
> There's an issue to remove this function since it's now unused.
Removed in the latest commit.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/25847#discussion_r2161282585
PR Review Comment: https://git.openjdk.org/jdk/pull/25847#discussion_r2161282345
PR Review Comment: https://git.openjdk.org/jdk/pull/25847#discussion_r2161282101
