8315061: Make LockingMode a product flag

[dcubed-ojdk]

A trivial fix to make LockingMode a product flag.

---

Progress

• Change must be properly reviewed (1 review required, with at least 1 Reviewer)
• Change must not contain extraneous whitespace
• Commit message must refer to an issue
• Change requires CSR request JDK-8315275 to be approved

Issues

• JDK-8315061: Make LockingMode a product flag (Enhancement - P4)
• JDK-8315275: Make LockingMode a product flag (CSR)

Reviewers

• Calvin Cheung (@calvinccheung - Reviewer)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/15496/head:pull/15496
$ git checkout pull/15496

Update a local copy of the PR:
$ git checkout pull/15496
$ git pull https://git.openjdk.org/jdk.git pull/15496/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 15496

View PR using the GUI difftool:
$ git pr show -t 15496

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/15496.diff

Webrev

Link to Webrev Comment

[dcubed-ojdk]

/label add hotspot-runtime

[bridgekeeper]

👋 Welcome back dcubed! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

[openjdk]

@dcubed-ojdk
The hotspot-runtime label was successfully added.

[mlbridge]

Webrevs

• 00: Full (fd0dae7c)

[calvinccheung]

Looks good and seems trivial.

[openjdk]

@dcubed-ojdk This change now passes all automated pre-integration checks.

ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details.

After integration, the commit message for the final commit will be:

8315061: Make LockingMode a product flag

Reviewed-by: ccheung

You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed.

At the time when this comment was updated there had been 2 new commits pushed to the master branch:

• c90cd2c: 8286789: Test forceEarlyReturn002.java timed out
• 89d18ea: 8312018: Improve reservation of class space and CDS

Please see this link for an up-to-date comparison between the source branch of this pull request and the master branch.
As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details.

➡️ To integrate this PR with the above commit message to the master branch, type /integrate in a new comment.

[dcubed-ojdk]

@calvinccheung - Thanks for the fast review.

[dcubed-ojdk]

Verified with a local fastdebug build without enabling experimental options:

$ build/macosx-x86_64-normal-server-fastdebug/images/jdk/bin/java -XX:LockingMode=2 -version
java version "22-internal" 2024-03-19
Java(TM) SE Runtime Environment (fastdebug build 22-internal-2023-08-30-1953507.dcubed...)
Java HotSpot(TM) 64-Bit Server VM (fastdebug build 22-internal-2023-08-30-1953507.dcubed..., mixed mode, sharing)

$ build/macosx-x86_64-normal-server-fastdebug/images/jdk/bin/java -XX:LockingMode=3 -version
int LockingMode=3 is outside the allowed range [ 0 ... 2 ]
Improperly specified VM option 'LockingMode=3'
Error: Could not create the Java Virtual Machine.
Error: A fatal exception has occurred. Program will exit.

[dcubed-ojdk]

/integrate

[openjdk]

Going to push as commit 3eac890.
Since your change was applied there have been 3 commits pushed to the master branch:

• 8419a53: 8315072: Remove unneeded AdaptivePaddedAverage::operator new
• c90cd2c: 8286789: Test forceEarlyReturn002.java timed out
• 89d18ea: 8312018: Improve reservation of class space and CDS

Your commit was automatically rebased without conflicts.

[openjdk]

@dcubed-ojdk Pushed as commit 3eac890.

💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.

[rkennke]

I'm sorry, I probably still don't understand. Shouldn't the flag be a diagnostic flag? The default value of the flag is what should be used, and the flag should only be changed if something doesn't work and when it is suspected that it's the locking implementation that's problematic.

[dcubed-ojdk]

Repeating my answer from the RFE:

https://bugs.openjdk.org/browse/JDK-8315061?focusedCommentId=14607768&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-14607768

In order to change the default LockingMode value from LM_LEGACY to
LM_LIGHTWEIGHT, we have to first change LockingMode to a product
flag in JDK22 and then in the next release we can change the default
LockingMode value from LM_LEGACY to LM_LIGHTWEIGHT.

We can't change the default LockingMode value without making LockingMode
into a product flag because we have to give customers a supported means
of changing the LockingMode should there be any issues with the new default.

[rkennke]

Repeating my answer from the RFE:

https://bugs.openjdk.org/browse/JDK-8315061?focusedCommentId=14607768&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-14607768

In order to change the default LockingMode value from LM_LEGACY to

LM_LIGHTWEIGHT, we have to first change LockingMode to a product

flag in JDK22 and then in the next release we can change the default

LockingMode value from LM_LEGACY to LM_LIGHTWEIGHT.

We can't change the default LockingMode value without making LockingMode

into a product flag because we have to give customers a supported means

of changing the LockingMode should there be any issues with the new default.

Hmmm ok. It already has been a product flag, but it has been experimental. Requiring to unlock experimental or diagnostic options for customers to change the flag is not enough to change the default value, then? I was not aware of that...

[dcubed-ojdk]

It was an "experimental" product flag and many customers won't use experimental
product flags. We see similar push back on "diagnostic" product flags, but sometimes
customers are willing to temporarily use a diagnostic flag in their production setups
to help us diagnose a failure that only happens in production.

I hope all this clarifies why I had to change LockingMode to a straight product flag.

[dholmes-ora]

It can be confusing to refer to experimental and diagnostic flags as "product" flags, in the sense that they are not "develop" flags, but they are not a full-fledged "product flag" which is a non-develop flag that is neither experimental nor diagnostic. We refer to the latter simply as "product flags".

By making this a product flag we also potentially increase the size of the user base that might be willing to test the new locking mode in their systems. We need to have some confidence that the new code has actually been used outside OpenJDK development and testing before making it the default for everyone. This change to full product flag is a necessary, but not in itself sufficient, step towards that.

[rkennke]

Ok, alright, thanks for the clarifications!