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

8259380: Correct pretouch chunk size to cap with actual page size #1978

Closed
wants to merge 1 commit into from

Conversation

cnqpzhang
Copy link
Contributor

@cnqpzhang cnqpzhang commented Jan 7, 2021

This is actually a regression, with regards to JVM startup time extreme slowdown, initially found at an aarch64 platform (Ampere Altra core).

The chunk size of pretouching should cap with the input page size which probably stands for large pages size if UseLargePages was set, otherwise processing chunks with much smaller size inside large size pages would hurt performance.

This issue was introduced during a refactor on chunk calculations JDK-8254972 (2c7fc85) but did not cause any problem immediately since the default PreTouchParallelChunkSize for all platforms are 1GB which can cover all popular sizes of large pages in use by most kernel variations. Later on, JDK-8254699 (805d058) set default 4MB for Linux platform, which is helpful to speed up startup time for some platforms. For example, most x64, since the popular default large page size (e.g. CentOS) is 2MB. In contrast, most default large page size with aarch64 platforms/kernels (e.g. CentOS) are 512MB, so using the 4MB chunk size to do page walk through the pages inside 512MB large page hurt performance of startup time.

In addition, there will be a similar problem if we set -XX:PreTouchParallelChunkSize=4k at a x64 Linux platform, the startup slowdown will show as well.

Tests:
https://bugs.openjdk.java.net/secure/attachment/92623/pretouch_chunk_size_fix_testing.txt
The 4 before-after comparisons show the JVM startup time go back to normal.
1). 33.381s to 0.870s
2). 20.333s to 2.740s
3). 15.090s to 6.268s
4). 38.983s to 6.709s
(Use the start time of pretouching the first Survivor space as a rough measurement, while \time, or GCTraceTime can generate similar results)


Progress

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

Issue

  • JDK-8259380: Correct pretouch chunk size to cap with actual page size

Download

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

@bridgekeeper
Copy link

bridgekeeper bot commented Jan 7, 2021

👋 Welcome back qpzhang! 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 the rfr Pull request is ready for review label Jan 7, 2021
@openjdk
Copy link

openjdk bot commented Jan 7, 2021

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

  • hotspot-gc

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.

@openjdk openjdk bot added the hotspot-gc hotspot-gc-dev@openjdk.org label Jan 7, 2021
@mlbridge
Copy link

mlbridge bot commented Jan 7, 2021

Webrevs

@openjdk
Copy link

openjdk bot commented Jan 8, 2021

@cnqpzhang The label aarch64 is not a valid label. These labels are valid:

  • serviceability
  • hotspot
  • sound
  • hotspot-compiler
  • kulla
  • i18n
  • shenandoah
  • jdk
  • javadoc
  • 2d
  • security
  • swing
  • hotspot-runtime
  • jmx
  • build
  • nio
  • beans
  • core-libs
  • compiler
  • net
  • hotspot-gc
  • hotspot-jfr
  • awt

@cnqpzhang
Copy link
Contributor Author

/label add hotspot

@openjdk openjdk bot added the hotspot hotspot-dev@openjdk.org label Jan 8, 2021
@openjdk
Copy link

openjdk bot commented Jan 8, 2021

@cnqpzhang
The hotspot label was successfully added.

@tschatzl
Copy link
Contributor

tschatzl commented Jan 8, 2021

Thanks for finding and reporting this issue and even providing a patch.

After having looked at the issue we (in the Oracle GC team) think this problem is serious enough to actually go into JDK16. Since backporting after having this pushed to some (this) repo is some extra effort, would you mind closing this PR here on openjdk/jdk and reopening a new one on openjdk/jdk16?

It will then be automatically forward ported to this repo. Not only is backporting some additional effort, there is concern that it won't make it into jdk16 otherwise - Jan 14 is cutoff date for bugs of this seriousness, and we'd need to get an exception for this otherwise.

As one of the persons typically triaging new issues in the bug tracker, I would also like to ask you to not open new issues immediately. We are looking at these issues three times a week, and if you open them yourselves, issues might not be handled correctly (i.e. like in this case immediately put into openjdk/jdk16). You can still create a PR and everything even if an issue is in "New" state.

I'll start looking at your change immediately.

Thanks,
Thomas

@tschatzl
Copy link
Contributor

tschatzl commented Jan 8, 2021

Fwiw, the most appropriate label for this change would probably be "hotspot-gc", but "hotspot" is fine too.

@cnqpzhang
Copy link
Contributor Author

cnqpzhang commented Jan 8, 2021

Understood, I will do this today.

would you mind closing this PR here on openjdk/jdk and reopening a new one on openjdk/jdk16?

OK. Can I use the same tracker (8259380 for the PR to jdk16?

As one of the persons typically triaging new issues in the bug tracker, I would also like to ask you to not open new issues immediately.

@cnqpzhang
Copy link
Contributor Author

/label remove hotspot

@tschatzl
Copy link
Contributor

tschatzl commented Jan 8, 2021

Yes, keep everything the same, the only difference is to create a pull request for openjdk/jdk16, not openjdk/jdk.

@openjdk openjdk bot removed the hotspot hotspot-dev@openjdk.org label Jan 8, 2021
@openjdk
Copy link

openjdk bot commented Jan 8, 2021

@cnqpzhang
The hotspot label was successfully removed.

@cnqpzhang
Copy link
Contributor Author

Yes, keep everything the same, the only difference is to create a pull request for openjdk/jdk16, not openjdk/jdk.

Done. Please review the copied openjdk/jdk16#97, and I am going to close this as suggested. Thanks. @tschatzl

@tschatzl
Copy link
Contributor

tschatzl commented Jan 8, 2021

Saw it. Thanks.

@cnqpzhang cnqpzhang closed this Jan 8, 2021
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 rfr Pull request is ready for review
Development

Successfully merging this pull request may close these issues.

2 participants