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

Fix code cache allocation with large pages enabled #18342

Merged
merged 1 commit into from
Nov 1, 2023

Conversation

SajinaKandy
Copy link
Contributor

Code cache allocation sizes are calculated and rounded up to the page size available on the system. When large pages/huge pages are enabled, and page size >= 1GB, this can cause the code cache size calculation to go wrong.
When codecachetotal is lesser than the page size, it makes the size of code cache segment bigger than the total code cache size (default 256MB for a 64-bit VM).
If large pages are enabled, page size should be recalculated to a smaller value if required. Also a warning message should be printed in the verbose output.

Closes: ##18274

@SajinaKandy
Copy link
Contributor Author

On a machine with 2M large page size,
java -Xcodecachetotal1M -Xcodecache1M "-Xjit:verbose={codecache,compilePerformance}" MemoryUsageTest

will output the following in the verbose output:

#CODECACHE:  Warning:Using page size 4096 instead of large page size 2097152
#CODECACHE:  The code cache repository was allocated between addresses 00007FC98DE00000 and 00007FC991E00000 to be near the VM/JIT modules to avoid trampolines
#CODECACHE:  allocated code cache segment of size 1048576
#CODECACHE:  allocateCodeCacheRepository: size=1048576 heapBase=00007FC990251000 heapAlloc=00007FC990251008 heapTop=00007FC990351000
#CODECACHE:  carved size=1048568 range: 00007FC990251008-00007FC990351000
#CODECACHE:  CodeCache allocated 00007FC98C0CCEB0 @ 00007FC990251008-00007FC990351000 HelperBase:00007FC990350260

The output of the memory usage for JIT Code cache is:

Name: JIT code cache
Type: Non-heap memory
Usage: init = 1048576(1024K) used = 1041568(1017K) committed = 1048576(1024K) max = 1048576(1024K)
Peak Usage: init = 1048576(1024K) used = 1048000(1023K) committed = 1048576(1024K) max = 1048576(1024K)
Collection Usage: null

In order to do this test I removed the lines which imposes a 2MB minimum on the code cache total size:

if (ccTotalSize < 2048)
ccTotalSize = 2048;

Next Action: Test on a machine with 1GB huge page size.

@SajinaKandy SajinaKandy marked this pull request as draft October 25, 2023 01:55
@SajinaKandy SajinaKandy changed the title Fix code cache allocation with large pages enabled WIP:Fix code cache allocation with large pages enabled Oct 25, 2023
@mpirvu mpirvu self-assigned this Oct 27, 2023
Copy link
Contributor

@mpirvu mpirvu left a comment

Choose a reason for hiding this comment

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

See the inline comments

runtime/compiler/runtime/J9CodeCacheManager.cpp Outdated Show resolved Hide resolved
runtime/compiler/runtime/J9CodeCacheManager.cpp Outdated Show resolved Hide resolved
@mpirvu
Copy link
Contributor

mpirvu commented Oct 27, 2023

For me this PR does not build locally.

/home/mpirvu/FullJava11/openj9-openjdk-jdk11/openj9/runtime/gc_modron_startup/mmparseXXgc.cpp:645:31: error: 'class MM_Configuration' has no member named '_packetListSplitForced'
  645 |    extensions->configuration->_packetListSplitForced = true;

It's possible that there is a mismatch between this openj9 code and the omr one.
@SajinaKandy please rebase this branch.

@mpirvu
Copy link
Contributor

mpirvu commented Oct 28, 2023

The code works at least when 4KB and 1GB pages are available.
I'll see if I can get 4KB, 2 MB and 1GB pages all at the same time.

@SajinaKandy SajinaKandy force-pushed the huge_page_fix branch 2 times, most recently from 76f5e42 to 9ed2d87 Compare October 29, 2023 01:08
@SajinaKandy
Copy link
Contributor Author

@mpirvu Its great you are able to test this. I have rebased my work.

@mpirvu
Copy link
Contributor

mpirvu commented Oct 30, 2023

Since not many people have verbose logs enabled, I think we should also add a trace point for this rare case (basically under the same conditions the verbose line is printed).
Moreover, this change needs to be documented.

@mpirvu
Copy link
Contributor

mpirvu commented Oct 30, 2023

I cannot get both 2MB and 1GB large pages enabled on the system. It's either one or the other.
The proposed code should fix the problem seen by the customer.

@SajinaKandy SajinaKandy force-pushed the huge_page_fix branch 2 times, most recently from c10b899 to 749017d Compare October 31, 2023 12:32
@SajinaKandy SajinaKandy changed the title WIP:Fix code cache allocation with large pages enabled Fix code cache allocation with large pages enabled Oct 31, 2023
When large pages/huge pages are enabled and page size >= 1GB, code cache
size is rounded up to the page size. When codecachetotal is lesser than
the page size, this can make the size of code cache segment bigger
than the total code cache size.
In this case, page size should be recalculated to a smaller value.
Also a warning message should be printed in the verbose output.

Closes: #eclipse-openj9#18274
Signed-off-by: SajinaKandy <sajina.kandy@ibm.com>
@SajinaKandy SajinaKandy marked this pull request as ready for review October 31, 2023 23:27
Copy link
Contributor

@mpirvu mpirvu left a comment

Choose a reason for hiding this comment

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

LGTM

@mpirvu
Copy link
Contributor

mpirvu commented Nov 1, 2023

jenkins compile all jdk21

@mpirvu
Copy link
Contributor

mpirvu commented Nov 1, 2023

jenkins compile alinux64 jdk21

@mpirvu
Copy link
Contributor

mpirvu commented Nov 1, 2023

jenkins test sanity zlinux jdk17

@mpirvu mpirvu merged commit 032fb02 into eclipse-openj9:master Nov 1, 2023
10 checks passed
@mpirvu
Copy link
Contributor

mpirvu commented Nov 1, 2023

@SajinaKandy don't forget to create an issue at https://github.com/eclipse-openj9/openj9-docs to describe the change in behavior when large pages are used.

@SajinaKandy
Copy link
Contributor Author

@mpirvu Raised the doc issue: eclipse-openj9/openj9-docs#1206

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants