8338136: Hotspot should support multiple large page sizes on Windows #20758

dhanalla · 2024-08-29T00:18:04Z

This pull request introduces enhancements to the handling of large page sizes in the OpenJDK for Windows systems, aiming to align its capabilities with those observed on Linux platforms. Investigation through SPECJBB benchmarks across various platforms revealed a 16-year-old limitation in handling large pages over 4MB for IA32/AMD64 architectures, with no such constraints for Windows on ARM64.

JBS issue https://bugs.openjdk.org/browse/JDK-8338136

The goal of this change is to overcome the 4MB large page size limitation, thereby enhancing Windows' large page support to match Linux's more flexible handling capabilities. This decision to remove 4MB constraint was influenced by insights from Linux's implementation strategies. The implementation supports multiple large page sizes , specifically excluding the IA32 architecture.

Key changes and bug fixes include enabling Windows support for multiple huge page sizes with -XX:LargePageSizeInBytes, and utilizing logic from JDK-8271195 to use the largest available large page size smaller than LargePageSizeInBytes when available.

This update removes the 4MB limit on AMD64 for Windows, populating the shared array to enable fallback options on all architectures except IA32. The implementation introduces an experimental flag, defaulting to FALSE, to facilitate testing and gradual adoption of these changes. The flag allows users to opt-in to the new large page handling logic, with a warning mechanism implemented for cases where the requested large page size is not a multiple of the OS minimum page size.

Progress

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

Issues

JDK-8338136: Hotspot should support multiple large page sizes on Windows (Enhancement - P3)
JDK-8339798: Hotspot should support multiple large page sizes on Windows (CSR)

Reviewers

David Holmes (@dholmes-ora - Reviewer)
Daniel Jeliński (@djelinski - Reviewer)

Reviewing

Using git

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

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

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 20758

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

Using diff file

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

Webrev

Link to Webrev Comment

bridgekeeper · 2024-08-29T00:18:28Z

👋 Welcome back dhanalla! 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 · 2024-08-29T00:18:59Z

@dhanalla 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:

8338136: Hotspot should support multiple large page sizes on Windows

Reviewed-by: dholmes, djelinski

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 479 new commits pushed to the master branch:

10402b4: 8341489: ProblemList runtime/cds/appcds/DumpRuntimeClassesTest.java in Xcomp mode
6bc3971: 8341316: [macos] javax/swing/ProgressMonitor/ProgressMonitorEscapeKeyPress.java fails sometimes in macos
e89fd1d: 8341128: open source some 2d graphics tests
6f459af: 8340077: Open source few Checkbox tests - Set2
b6e72ff: 8339403: sun.security.ssl.StatusResponseManager.get swallows interrupt status
013250e: 8340432: Open source some MenuBar tests - Set2
de12fc7: 8339684: ResizeObserver callback interrupts smooth scrolling on Chrome
ebb4759: 8340625: Open source additional Component tests (part 3)
3ee94e0: 8341282: (fs) Move creation time fallback logic to Java layer (Linux)
f1ea57f: 8340229: Improve opening sentence of FileInputStream constructor specification
... and 469 more: https://git.openjdk.org/jdk/compare/26e3d535ad4d6e5d78ca50941cfa39dd337892a9...master

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.

As you do not have Committer status in this project an existing Committer must agree to sponsor your change. Possible candidates are the reviewers of this PR (@dholmes-ora, @djelinski) but any other Committer may sponsor as well.

➡️ To flag this PR as ready for integration with the above commit message, type /integrate in a new comment. (Afterwards, your sponsor types /sponsor in a new comment to perform the integration).

openjdk · 2024-08-29T00:19:22Z

@dhanalla The following label will be automatically applied to this pull request:

hotspot-runtime

When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing list. If you would like to change these labels, use the /label pull request command.

mlbridge · 2024-08-29T00:22:43Z

Webrevs

dholmes-ora

Hi @dhanalla ,

What criteria do you think would suffice for EnableAllLargePageSizesForWindows to default to true or else be removed altogether, in the future?

dhanalla · 2024-09-03T00:34:39Z

Hi @dhanalla ,

What criteria do you think would suffice for EnableAllLargePageSizesForWindows to default to true or else be removed altogether, in the future?

Thanks @dholmes-ora for reviewing this PR.

To determine the best time to set EnableAllLargePageSizesForWindows to true by default or consider its removal, we first need to gather customer feedback. Once we have confirmed its effectiveness, we can assess the option of making it the default setting. At that stage, the -XX:+UseLargePages flag should adequately manage all large page scenarios.

dholmes-ora · 2024-09-03T04:22:23Z

@dhanalla do you think your customers will be okay running with an experimental flag enabled?

The changes seem fine. I'm running them through our CI with the flag disabled and enabled to see if anything interesting happens.

dholmes-ora · 2024-09-03T06:23:26Z

The interesting tests were skipped:

Skipping test because lock memory privilege is not granted

what do you need to make the "lock memory privilege" available?

EDIT: never mind I found it.

dhanalla · 2024-09-03T17:16:45Z

The interesting tests were skipped:
Skipping test because lock memory privilege is not granted
what do you need to make the "lock memory privilege" available?

EDIT: never mind I found it.

Thanks for running through CI.
If you're still looking for the option, here's the path in gpedit.msc to configure the "Lock pages in memory" setting. To add a user or user group:
Computer Configuration > Windows Settings > Security Settings > Local Policies > User Rights Assignment > "Lock pages in memory"

dhanalla · 2024-09-06T18:30:19Z

@dhanalla do you think your customers will be okay running with an experimental flag enabled?

The changes seem fine. I'm running them through our CI with the flag disabled and enabled to see if anything interesting happens.

@dholmes-ora, Could you please advise if we can remove the 'experimental' designation for EnableAllLargePageSizesForWindows and keep it set to false by default for now, with the possibility of changing it to true by default or removing it at a later date?

dholmes-ora · 2024-09-06T20:27:25Z

If you want the flag to be a full blown product flag then you just need to create a CSR request and get that approved.

BTW I'm still trying to get a test environment where we have the lock-pages-in-memory privilege to test this.

dhanalla · 2024-09-20T21:30:19Z

If you want the flag to be a full blown product flag then you just need to create a CSR request and get that approved.

BTW I'm still trying to get a test environment where we have the lock-pages-in-memory privilege to test this.

@dholmes-ora, May I know if the test is complete and if the results are as expected?

dholmes-ora · 2024-09-23T01:37:38Z

Sorry @dhanalla we are having some technical problem getting a test environment configured.

I will approve now but you still need a second reviewer. If any issues arise we will deal with them at the time.

djelinski

I'm not familiar with JVM's memory handling, so just a few general observations...

djelinski · 2024-10-01T09:15:46Z

src/hotspot/os/windows/os_windows.cpp

+      WARN("The specified large page size (%d) is not a multiple of the minimum large page size (%d), defaulting to minimum page size.", LargePageSizeInBytes, size);
+    }
+  } else {
+    WARN("The JVM cannot use large pages because the large page size setting is not configured, defaulting to minimum page size (%d).", size);


This message looks wrong; LargePageSizeInBytes=0 is documented to use the default large page size, and it seems that the surrounding code does just that. Is the warning even necessary?

You are correct; the warning message does seem misleading. The LargePageSizeInBytes=0 is meant to use the default large page size, which the surrounding code already handles. Therefore, the warning about large pages not being configured is unnecessary.
We can remove this warning and adjust the code to reflect the intended use of default large page sizes when
LargePageSizeInBytes is zero.

djelinski · 2024-10-01T09:21:22Z

src/hotspot/os/windows/os_windows.cpp

-#if defined(IA32) || defined(AMD64)
-  if (size > 4*M || LargePageSizeInBytes > 4*M) {
+#if defined(IA32)
+    if (size > 4 * M || LargePageSizeInBytes > 4 * M) {


Please fix the indentation

Thank you @djelinski for reviewing this PR.
fixed the indentation.

djelinski · 2024-10-01T09:24:27Z

src/hotspot/os/windows/os_windows.cpp

+      size_t min_size = GetLargePageMinimum();
+
+      // Populate _page_sizes with large page sizes less than or equal to _large_page_size, ensuring each page size is double the size of the previous one.
+      for (size_t page_size = min_size; page_size < _large_page_size; page_size *= 2) {


Does Windows actually use these page sizes, or does it always use 2M pages?

Also, you're only adding powers of 2 here; the user-defined LargePageSizeInBytes doesn't need to be a power of two, and can be 6MB, for example. Will that be a problem?

Windows can use different large page sizes based on hardware support, not limited to the default 2MB. The implementation examines the system's supported page sizes and selects an appropriate size using the LargePageSizeInBytes parameter.
The code accommodates non-power-of-two sizes, such as 6MB, while ensuring these sizes are multiples of the system's minimum large page size to prevent potential issues. If an invalid size is specified, the code defaults to the minimum supported size and issues a warning.

djelinski

LGTM.

dhanalla · 2024-10-03T21:18:07Z

/integrate

openjdk · 2024-10-03T21:19:39Z

@dhanalla
Your change (at version f778475) is now ready to be sponsored by a Committer.

dholmes-ora · 2024-10-04T04:49:57Z

/sponsor

openjdk · 2024-10-04T04:50:56Z

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

10402b4: 8341489: ProblemList runtime/cds/appcds/DumpRuntimeClassesTest.java in Xcomp mode
6bc3971: 8341316: [macos] javax/swing/ProgressMonitor/ProgressMonitorEscapeKeyPress.java fails sometimes in macos
e89fd1d: 8341128: open source some 2d graphics tests
6f459af: 8340077: Open source few Checkbox tests - Set2
b6e72ff: 8339403: sun.security.ssl.StatusResponseManager.get swallows interrupt status
013250e: 8340432: Open source some MenuBar tests - Set2
de12fc7: 8339684: ResizeObserver callback interrupts smooth scrolling on Chrome
ebb4759: 8340625: Open source additional Component tests (part 3)
3ee94e0: 8341282: (fs) Move creation time fallback logic to Java layer (Linux)
f1ea57f: 8340229: Improve opening sentence of FileInputStream constructor specification
... and 469 more: https://git.openjdk.org/jdk/compare/26e3d535ad4d6e5d78ca50941cfa39dd337892a9...master

Your commit was automatically rebased without conflicts.

openjdk · 2024-10-04T04:51:02Z

@dholmes-ora @dhanalla Pushed as commit 4ded283.

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

dhanalla · 2024-11-08T01:11:57Z

/backport jdk23u

openjdk · 2024-11-08T01:12:55Z

@dhanalla the backport was successfully created on the branch backport-dhanalla-4ded2838-master in my personal fork of openjdk/jdk23u. To create a pull request with this backport targeting openjdk/jdk23u:master, just click the following link:

➡️ Create pull request

The title of the pull request is automatically filled in correctly and below you find a suggestion for the pull request body:

Hi all,

This pull request contains a backport of commit 4ded2838 from the openjdk/jdk repository.

The commit being backported was authored by Dhamoder Nalla on 4 Oct 2024 and was reviewed by David Holmes and Daniel Jeliński.

Thanks!

If you need to update the source branch of the pull then run the following commands in a local clone of your personal fork of openjdk/jdk23u:

$ git fetch https://github.com/openjdk-bots/jdk23u.git backport-dhanalla-4ded2838-master:backport-dhanalla-4ded2838-master
$ git checkout backport-dhanalla-4ded2838-master
# make changes
$ git add paths/to/changed/files
$ git commit --message 'Describe additional changes made'
$ git push https://github.com/openjdk-bots/jdk23u.git backport-dhanalla-4ded2838-master

⚠️ @dhanalla You are not yet a collaborator in my fork openjdk-bots/jdk23u. An invite will be sent out and you need to accept it before you can proceed.

dhanalla · 2025-03-25T16:42:36Z

/backport jdk21u-dev

openjdk · 2025-03-25T16:43:32Z

@dhanalla the backport was successfully created on the branch backport-dhanalla-4ded2838-master in my personal fork of openjdk/jdk21u-dev. To create a pull request with this backport targeting openjdk/jdk21u-dev:master, just click the following link:

➡️ Create pull request

The title of the pull request is automatically filled in correctly and below you find a suggestion for the pull request body:

Hi all,

This pull request contains a backport of commit 4ded2838 from the openjdk/jdk repository.

The commit being backported was authored by Dhamoder Nalla on 4 Oct 2024 and was reviewed by David Holmes and Daniel Jeliński.

Thanks!

If you need to update the source branch of the pull then run the following commands in a local clone of your personal fork of openjdk/jdk21u-dev:

$ git fetch https://github.com/openjdk-bots/jdk21u-dev.git backport-dhanalla-4ded2838-master:backport-dhanalla-4ded2838-master
$ git checkout backport-dhanalla-4ded2838-master
# make changes
$ git add paths/to/changed/files
$ git commit --message 'Describe additional changes made'
$ git push https://github.com/openjdk-bots/jdk21u-dev.git backport-dhanalla-4ded2838-master

⚠️ @dhanalla You are not yet a collaborator in my fork openjdk-bots/jdk21u-dev. An invite will be sent out and you need to accept it before you can proceed.

Hotspot should support multiple large page sizes on Windows

36bb5f2

openjdk bot added the rfr Pull request is ready for review label Aug 29, 2024

openjdk bot added the hotspot-runtime hotspot-runtime-dev@openjdk.org label Aug 29, 2024

dholmes-ora reviewed Aug 29, 2024

View reviewed changes

dhanalla mentioned this pull request Aug 29, 2024

Hotspot should support multiple large page sizes on Windows microsoft/openjdk-jdk#23

Open

openjdk bot added csr Pull request needs approved CSR before integration and removed csr Pull request needs approved CSR before integration labels Sep 9, 2024

Make JVM flag EnableAllLargePageSizesForWindows non-experimental

d48aff3

dholmes-ora approved these changes Sep 23, 2024

View reviewed changes

openjdk bot added the ready Pull request is ready to be integrated label Sep 23, 2024

djelinski reviewed Oct 1, 2024

View reviewed changes

address CR feedback

f778475

openjdk bot removed the ready Pull request is ready to be integrated label Oct 3, 2024

dholmes-ora approved these changes Oct 3, 2024

View reviewed changes

openjdk bot added the ready Pull request is ready to be integrated label Oct 3, 2024

djelinski approved these changes Oct 3, 2024

View reviewed changes

openjdk bot added the sponsor Pull request is ready to be sponsored label Oct 3, 2024

openjdk bot added the integrated Pull request has been integrated label Oct 4, 2024

openjdk bot closed this Oct 4, 2024

openjdk bot removed ready Pull request is ready to be integrated rfr Pull request is ready for review sponsor Pull request is ready to be sponsored labels Oct 4, 2024

dhanalla deleted the JDK_8338136 branch November 8, 2024 01:12

dhanalla mentioned this pull request Nov 8, 2024

8338136: Hotspot should support multiple large page sizes on Windows openjdk/jdk23u#219

Closed

4 tasks

8338136: Hotspot should support multiple large page sizes on Windows #20758

8338136: Hotspot should support multiple large page sizes on Windows #20758

Uh oh!

Conversation

dhanalla commented Aug 29, 2024 • edited by openjdk bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Progress

Issues

Reviewers

Reviewing

Webrev

Uh oh!

bridgekeeper bot commented Aug 29, 2024

Uh oh!

openjdk bot commented Aug 29, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

openjdk bot commented Aug 29, 2024

Uh oh!

mlbridge bot commented Aug 29, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Webrevs

Uh oh!

dholmes-ora left a comment

Choose a reason for hiding this comment

Uh oh!

dhanalla commented Sep 3, 2024

Uh oh!

dholmes-ora commented Sep 3, 2024

Uh oh!

dholmes-ora commented Sep 3, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dhanalla commented Sep 3, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dhanalla commented Sep 6, 2024

Uh oh!

dholmes-ora commented Sep 6, 2024

Uh oh!

dhanalla commented Sep 20, 2024

Uh oh!

dholmes-ora commented Sep 23, 2024

Uh oh!

djelinski left a comment

Choose a reason for hiding this comment

Uh oh!

djelinski Oct 1, 2024

Choose a reason for hiding this comment

Uh oh!

dhanalla Oct 3, 2024

Choose a reason for hiding this comment

Uh oh!

djelinski Oct 1, 2024

Choose a reason for hiding this comment

Uh oh!

dhanalla Oct 3, 2024

Choose a reason for hiding this comment

Uh oh!

djelinski Oct 1, 2024

Choose a reason for hiding this comment

Uh oh!

dhanalla Oct 3, 2024

Choose a reason for hiding this comment

Uh oh!

djelinski left a comment

Choose a reason for hiding this comment

Uh oh!

dhanalla commented Oct 3, 2024

Uh oh!

openjdk bot commented Oct 3, 2024

Uh oh!

dholmes-ora commented Oct 4, 2024

Uh oh!

openjdk bot commented Oct 4, 2024

Uh oh!

openjdk bot commented Oct 4, 2024

Uh oh!

dhanalla commented Nov 8, 2024

Uh oh!

dhanalla commented Aug 29, 2024 •

edited by openjdk bot

Loading

openjdk bot commented Aug 29, 2024 •

edited

Loading

mlbridge bot commented Aug 29, 2024 •

edited

Loading

dholmes-ora commented Sep 3, 2024 •

edited

Loading

dhanalla commented Sep 3, 2024 •

edited

Loading