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.

Sign up for GitHub

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

Sets recommended max Xmx 32G when set_recommended_max_xmx experiment is enabled #28442

Merged
merged 3 commits into from
Sep 19, 2023
Merged

Sets recommended max Xmx 32G when set_recommended_max_xmx experiment is enabled #28442

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
17e042f
Sets recommended max Xmx 32G when set_recommended_max_xmx experiment …
arvindram03 Sep 13, 2023
8127753
Fixing lint errors
arvindram03 Sep 13, 2023
f254d97
Adding comment to reason why 32G is recomemnded value
arvindram03 Sep 18, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 10 additions & 4 deletions sdks/java/container/boot.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -159,8 +159,9 @@ func main() {
cp = append(cp, filepath.Join(dir, filepath.FromSlash(name)))
}

var setRecommendedMaxXmx = strings.Contains(options, "set_recommended_max_xmx")
args := []string{
"-Xmx" + strconv.FormatUint(heapSizeLimit(info), 10),
"-Xmx" + strconv.FormatUint(heapSizeLimit(info, setRecommendedMaxXmx), 10),
// ParallelGC the most adequate for high throughput and lower CPU utilization
// It is the default GC in Java 8, but not on newer versions
"-XX:+UseParallelGC",
Expand Down Expand Up @@ -266,9 +267,14 @@ func makePipelineOptionsFile(options string) error {
// it returns 70% of the physical memory on the machine. If it cannot determine
// that value, it returns 1GB. This is an imperfect heuristic. It aims to
// ensure there is memory for non-heap use and other overhead, while also not
// underutilizing the machine.
func heapSizeLimit(info *fnpb.ProvisionInfo) uint64 {
if size, err := syscallx.PhysicalMemorySize(); err == nil {
// underutilizing the machine. if set_recommended_max_xmx experiment is enabled,
// sets xmx to 32G. Under 32G JVM enables CompressedOops. CompressedOops
// utilizes memory more efficiently, and has positive impact on GC performance
// and cache hit rate.
func heapSizeLimit(info *fnpb.ProvisionInfo, setRecommendedMaxXmx bool) uint64 {
if setRecommendedMaxXmx {
return 32 << 30
Copy link
Contributor

@arunpandianp arunpandianp Sep 14, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

It'll be flexible if we allow passing the xmx value via the experiment.
eg: --experiments=set_recommended_xmx=<xmx bytes>

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We do not want to expose the plug too much with this change as well. Which later becomes hard to control. In future, might be a generic way to control all the jvm args which will be covering this as well.

Copy link
Contributor

Choose a reason for hiding this comment

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

could you please include the description regarding why 32G as a comment here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We set the Xmx to the recommended max value(32G) with set_recommended_max_xmx is enabled. Under 32G JVM enables CompressedOops. CompressedOops utilizes memory more efficiently, and has positive impact on GC performance and cache hit rate.

Copy link
Contributor

Choose a reason for hiding this comment

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

thanks, I mean code comment

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

} else if size, err := syscallx.PhysicalMemorySize(); err == nil {
return (size * 70) / 100
}
return 1 << 30
Expand Down
Loading