RFR: 8359437: Make users and test suite not able to set LockingMode flag
Coleen Phillimore
coleenp at openjdk.org
Mon Jun 23 14:45:47 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 have a request for some more deieting.
This looks great. Thank you!
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.
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.
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.
-------------
Changes requested by coleenp (Reviewer).
PR Review: https://git.openjdk.org/jdk/pull/25847#pullrequestreview-2940262141
Marked as reviewed by coleenp (Reviewer).
PR Review: https://git.openjdk.org/jdk/pull/25847#pullrequestreview-2949835857
PR Review Comment: https://git.openjdk.org/jdk/pull/25847#discussion_r2155234150
PR Review Comment: https://git.openjdk.org/jdk/pull/25847#discussion_r2155246651
PR Review Comment: https://git.openjdk.org/jdk/pull/25847#discussion_r2155256340
More information about the hotspot-dev
mailing list