Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

8252973: ZGC: Implement Large Pages support on Windows #1184

Closed

Conversation

stefank
Copy link
Member

@stefank stefank commented Nov 12, 2020

Please review this patch to add large pages support to ZGC on Windows.

The patch uses a shared AWE section to reserve, commit, and map memory. I've split the code into two implementations, one for the old small pages support, and one for the large pages support. The large pages implementation is chosen if the required APIs are available, memory locking privileges has been setup in Windows (standard requirement for enabling large pages), and the user has selected large pages.

Comparing the small and large pages implementations:

The AWE API requires that the AWE section is provided during the memory reservation stage, and also when committing memory. Previously, ZGC has not needed to share information between the virtual memory layer and the physical memory layer, so there's no straight forward way to pass the section over to the physical memory layer. To keep this dependency local to the Windows implementation, I've chose to use a global reference to the section.

There's no need to use the placeholder API. The placeholders were needed to atomically replace a memory reservation with committed memory and at the same time support memory mapping. The placeholders needed to be dynamically split and coalesced, and some extra infrastructure were added for that. This is not needed with the AWE APIs, and makes that implementation a bit easier to understand IMHO.

When committing memory, the small pages implementation creates one paging file handle per ZGranule (2MB). This allows us to commit/uncommit at arbitrary 2MB boundaries. In the AWE implementation, it's very similar, but whereas the previous implementation chose to use a 2MB boundary, the AWE requires us to keep track of on handle per physical page (2MB again).

The shared AWE memory APIs are available since Windows version 1809. Thanks to Microsoft (@mo-beck and the memory management team) for describing how these APIs could be used to implement the operations we require to support large pages on Windows.


Progress

  • Change must not contain extraneous whitespace
  • Commit message must refer to an issue
  • Change must be properly reviewed

Testing

Linux x64 Linux x86 Windows x64 macOS x64
Build ✔️ (5/5 passed) ✔️ (2/2 passed) ✔️ (2/2 passed) ✔️ (2/2 passed)
Test (tier1) ✔️ (9/9 passed) ✔️ (9/9 passed) ✔️ (9/9 passed) ✔️ (9/9 passed)

Issue

  • JDK-8252973: ZGC: Implement Large Pages support on Windows

Reviewers

Download

$ git fetch https://git.openjdk.java.net/jdk pull/1184/head:pull/1184
$ git checkout pull/1184

@stefank
Copy link
Member Author

stefank commented Nov 12, 2020

/label add hotspot-gc

@bridgekeeper
Copy link

bridgekeeper bot commented Nov 12, 2020

👋 Welcome back stefank! 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 openjdk bot added rfr Pull request is ready for review hotspot-gc hotspot-gc-dev@openjdk.org labels Nov 12, 2020
@openjdk
Copy link

openjdk bot commented Nov 12, 2020

@stefank
The hotspot-gc label was successfully added.

@mlbridge
Copy link

mlbridge bot commented Nov 12, 2020

Webrevs

Copy link
Contributor

@fisk fisk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me.

@stefank
Copy link
Member Author

stefank commented Nov 16, 2020

Thanks @fisk

@stefank stefank changed the title 8252973: ZGC: Windows large pages support 8252973: ZGC: Implement Large Pages support on Windows Nov 16, 2020
@openjdk
Copy link

openjdk bot commented Nov 16, 2020

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

8252973: ZGC: Implement Large Pages support on Windows

Reviewed-by: eosterlund, mbeckwit, pliden

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 79 new commits pushed to 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.

@openjdk openjdk bot added the ready Pull request is ready to be integrated label Nov 16, 2020
Copy link
Contributor

@mo-beck mo-beck left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • LGTM.
  • I wanted to add that over the weekend, I took the PR for a spin on my Windows+Arm64 server. I ran SPECJBB2015 in the default (HBIR_RT) and also with PRESET higher IR. Each was run 5 times using the JBB script :: Number of successive runs set NUM_OF_RUNS=5 and found the run-to-run variance to be less than 2% for all metrics.
  • I also tested this on an x86-64 with different older versions of Windows. And here's an output from one of them with Windows version < 1803:
    [0.018s][error][gc] Failed to lookup symbol: VirtualAlloc2 Error occurred during initialization of VM ZGC requires Windows version 1803 or later

@stefank
Copy link
Member Author

stefank commented Nov 17, 2020

Thanks for reviewing @mo-beck!

Copy link
Contributor

@pliden pliden left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good!

@stefank
Copy link
Member Author

stefank commented Nov 23, 2020

Thanks for reviewing!
/integrate

@openjdk openjdk bot closed this Nov 23, 2020
@openjdk openjdk bot added integrated Pull request has been integrated and removed ready Pull request is ready to be integrated rfr Pull request is ready for review labels Nov 23, 2020
@openjdk
Copy link

openjdk bot commented Nov 23, 2020

@stefank Since your change was applied there have been 177 commits pushed to the master branch:

  • e4a32be: 8251925: C2: RenaissanceStressTest fails with assert(!had_error): bad dominance
  • 1f32c11: 8256740: ZGC: Move closures out of zOopClosure files
  • 659aec8: 8256719: C1 flags that should have expired are still present
  • e06a683: 8256497: Zero: enable G1 and Shenandoah GCs
  • 037e49c: 8256670: Zero: enable compressed oops support back
  • d46f6f5: 8256523: Streamline Java SHA2 implementation
  • 1aa90ac: 8256822: runtime/logging/RedefineClasses.java fails with "Error: VM option 'Verbose' is develop and is available only in debug version of VM."
  • edf72f0: 8256824: test/langtools/tools/javac/diags/examples/InnerClassCantHaveStatic.java has a bad copyright
  • 9a19eb6: 8254105: allow static nested declarations
  • 14de791: 8255934: JConsole 14 and greater fails to connect to older JVM
  • ... and 167 more: https://git.openjdk.java.net/jdk/compare/432c387e21c4291d353a6b30a4bbc2e41c944c35...master

Your commit was automatically rebased without conflicts.

Pushed as commit 69c3470.

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

openjdk-notifier bot referenced this pull request Nov 23, 2020
Reviewed-by: eosterlund, mbeckwit, pliden
@stefank stefank deleted the 8252973_zgc_windows_large_pages branch November 26, 2020 09:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
hotspot-gc hotspot-gc-dev@openjdk.org integrated Pull request has been integrated
Development

Successfully merging this pull request may close these issues.

4 participants