From 0381bc3a72840ba7e046ca73e1e3e07f3c98841d Mon Sep 17 00:00:00 2001 From: Pritham Marupaka Date: Mon, 13 Nov 2023 17:42:05 -0500 Subject: [PATCH 01/25] Add the UseProcessorAwareInitialHeapPercentage and set the initial heap percentage to be max(50%, (75%*heap - 3*numProcessors)/heap) --- launchlib/config.go | 5 ++- launchlib/launcher.go | 38 +++++++++++++--- launchlib/memory.go | 71 ++++++++++++++++++++++++++++++ launchlib/memory_test.go | 95 ++++++++++++++++++++++++++++++++++++++++ 4 files changed, 201 insertions(+), 8 deletions(-) create mode 100644 launchlib/memory.go create mode 100644 launchlib/memory_test.go diff --git a/launchlib/config.go b/launchlib/config.go index f1f6c5d6..2eb85b6d 100644 --- a/launchlib/config.go +++ b/launchlib/config.go @@ -71,7 +71,10 @@ type CustomLauncherConfig struct { DisableContainerSupport bool `yaml:"dangerousDisableContainerSupport"` } -type ExperimentalLauncherConfig struct{} +type ExperimentalLauncherConfig struct { + // Also disables setting the active processor count JVM argument. + UseProcessorAwareInitialHeapPercentage bool `yaml:"useProcessorAwareInitialHeapPercentage"` +} type PrimaryCustomLauncherConfig struct { VersionedConfig `yaml:",inline"` diff --git a/launchlib/launcher.go b/launchlib/launcher.go index 8ab8829c..3ee466e6 100644 --- a/launchlib/launcher.go +++ b/launchlib/launcher.go @@ -31,6 +31,7 @@ const ( TemplateDelimsClose = "}}" // ExecPathBlackListRegex matches characters disallowed in paths we allow to be passed to exec() ExecPathBlackListRegex = `[^\w.\/_\-]` + BytesInMebibyte = 1048576 ) type ServiceCmds struct { @@ -279,8 +280,8 @@ func delim(str string) string { func createJvmOpts(combinedJvmOpts []string, customConfig *CustomLauncherConfig, logger io.WriteCloser) []string { if isEnvVarSet("CONTAINER") && !customConfig.DisableContainerSupport && !hasMaxRAMOverride(combinedJvmOpts) { _, _ = fmt.Fprintln(logger, "Container support enabled") - combinedJvmOpts = filterHeapSizeArgs(combinedJvmOpts) - combinedJvmOpts = ensureActiveProcessorCount(combinedJvmOpts, logger) + combinedJvmOpts = filterHeapSizeArgs(customConfig, combinedJvmOpts) + combinedJvmOpts = ensureActiveProcessorCount(customConfig, combinedJvmOpts, logger) return combinedJvmOpts } @@ -295,7 +296,7 @@ func createJvmOpts(combinedJvmOpts []string, customConfig *CustomLauncherConfig, return combinedJvmOpts } -func filterHeapSizeArgs(args []string) []string { +func filterHeapSizeArgs(customConfig *CustomLauncherConfig, args []string) []string { var filtered []string var hasMaxRAMPercentage, hasInitialRAMPercentage bool for _, arg := range args { @@ -311,13 +312,17 @@ func filterHeapSizeArgs(args []string) []string { } if !hasInitialRAMPercentage && !hasMaxRAMPercentage { - filtered = append(filtered, "-XX:InitialRAMPercentage=75.0") - filtered = append(filtered, "-XX:MaxRAMPercentage=75.0") + initialHeapPercentage, err := computeInitialHeapPercentage(customConfig) + if err != nil { + initialHeapPercentage = 0.75 + } + filtered = append(filtered, fmt.Sprintf("-XX:InitialRAMPercentage=%f", initialHeapPercentage)) + filtered = append(filtered, fmt.Sprintf("-XX:MaxRAMPercentage=%f", initialHeapPercentage)) } return filtered } -func ensureActiveProcessorCount(args []string, logger io.Writer) []string { +func ensureActiveProcessorCount(customConfig *CustomLauncherConfig, args []string, logger io.Writer) []string { filtered := make([]string, 0, len(args)+1) var hasActiveProcessorCount bool @@ -328,7 +333,7 @@ func ensureActiveProcessorCount(args []string, logger io.Writer) []string { filtered = append(filtered, arg) } - if !hasActiveProcessorCount { + if !hasActiveProcessorCount && !customConfig.Experimental.UseProcessorAwareInitialHeapPercentage { processorCountArg, err := getActiveProcessorCountArg(logger) if err == nil { filtered = append(filtered, processorCountArg) @@ -376,3 +381,22 @@ func isMaxRAMPercentage(arg string) bool { func isInitialRAMPercentage(arg string) bool { return strings.HasPrefix(arg, "-XX:InitialRAMPercentage=") } + +func computeInitialHeapPercentage(customConfig *CustomLauncherConfig) (float64, error) { + if !customConfig.Experimental.UseProcessorAwareInitialHeapPercentage { + return 0.75, nil + } + + cgroupProcessorCount, err := DefaultCGroupV1ProcessorCounter.ProcessorCount() + if err != nil { + return 0, errors.Wrap(err, "failed to get cgroup processor count") + } + cgroupMemoryLimitInBytes, err := DefaultMemoryLimit.MemoryLimitInBytes() + if err != nil { + return 0, errors.Wrap(err, "failed to get cgroup memory limit") + } + var heapLimit = float64(cgroupMemoryLimitInBytes) + var processorOffset = 3 * BytesInMebibyte * float64(cgroupProcessorCount) + + return max(0.5, (0.75*heapLimit-processorOffset)/heapLimit), nil +} diff --git a/launchlib/memory.go b/launchlib/memory.go new file mode 100644 index 00000000..a0b756fc --- /dev/null +++ b/launchlib/memory.go @@ -0,0 +1,71 @@ +// Copyright 2023 Palantir Technologies, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package launchlib + +import ( + "io" + "io/fs" + "os" + "path/filepath" + "strconv" + "strings" + + "github.com/pkg/errors" +) + +const ( + memGroupName = "memory" + memLimitName = "memory.limit_in_bytes" +) + +type MemoryLimit interface { + MemoryLimitInBytes() (uint64, error) +} + +var DefaultMemoryLimit = NewCGroupMemoryLimit(os.DirFS("/")) + +type CGroupMemoryLimit struct { + pather CGroupPather + fs fs.FS +} + +func NewCGroupMemoryLimit(filesystem fs.FS) MemoryLimit { + return CGroupMemoryLimit{ + pather: NewCGroupV1Pather(filesystem), + fs: filesystem, + } +} + +func (c CGroupMemoryLimit) MemoryLimitInBytes() (uint64, error) { + memoryCGroupPath, err := c.pather.Path(memGroupName) + if err != nil { + return 0, errors.Wrap(err, "failed to get memory cgroup path") + } + + memLimitFilepath := filepath.Join(memoryCGroupPath, memLimitName) + memLimitFile, err := c.fs.Open(convertToFSPath(memLimitFilepath)) + if err != nil { + return 0, errors.Wrapf(err, "unable to open memory.limit_in_bytes at expected location: %s", memLimitFilepath) + } + memLimitBytes, err := io.ReadAll(memLimitFile) + if err != nil { + return 0, errors.Wrapf(err, "unable to read memory.limit_in_bytes") + } + memLimit, err := strconv.Atoi(strings.TrimSpace(string(memLimitBytes))) + if err != nil { + return 0, errors.New("unable to convert memory.limit_in_bytes value to expected type") + } + return uint64(memLimit), nil +} diff --git a/launchlib/memory_test.go b/launchlib/memory_test.go new file mode 100644 index 00000000..1e1527b7 --- /dev/null +++ b/launchlib/memory_test.go @@ -0,0 +1,95 @@ +// Copyright 2023 Palantir Technologies, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package launchlib_test + +import ( + "io/fs" + "testing" + "testing/fstest" + + "github.com/palantir/go-java-launcher/launchlib" + "github.com/pkg/errors" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +var ( + memoryLimitContent = []byte("2147483648\n") + badMemoryLimitContent = []byte(``) +) + +func TestMemoryLimit_DefaultMemoryLimit(t *testing.T) { + for _, test := range []struct { + name string + filesystem fs.FS + expectedMemoryLimit uint64 + expectedError error + }{ + { + name: "fails when unable to read memory.limit_in_bytes", + filesystem: fstest.MapFS{ + "proc/self/cgroup": &fstest.MapFile{ + Data: CGroupContent, + }, + "proc/self/mountinfo": &fstest.MapFile{ + Data: MountInfoContent, + }, + }, + expectedError: errors.New("unable to open memory.limit_in_bytes at expected location"), + }, + { + name: "fails when unable to parse memory.limit_in_bytes", + filesystem: fstest.MapFS{ + "proc/self/cgroup": &fstest.MapFile{ + Data: CGroupContent, + }, + "proc/self/mountinfo": &fstest.MapFile{ + Data: MountInfoContent, + }, + "sys/fs/cgroup/memory/memory.limit_in_bytes": &fstest.MapFile{ + Data: badMemoryLimitContent, + }, + }, + expectedError: errors.New("unable to convert memory.limit_in_bytes value to expected type"), + }, + { + name: "returns expected RAM percentage when memory.limit_in_bytes under 2 GiB", + filesystem: fstest.MapFS{ + "proc/self/cgroup": &fstest.MapFile{ + Data: CGroupContent, + }, + "proc/self/mountinfo": &fstest.MapFile{ + Data: MountInfoContent, + }, + "sys/fs/cgroup/memory/memory.limit_in_bytes": &fstest.MapFile{ + Data: memoryLimitContent, + }, + }, + expectedMemoryLimit: 1 << 31, + }, + } { + t.Run(test.name, func(t *testing.T) { + limit := launchlib.NewCGroupMemoryLimit(test.filesystem) + memoryLimit, err := limit.MemoryLimitInBytes() + if test.expectedError != nil { + require.Error(t, err) + assert.Contains(t, err.Error(), test.expectedError.Error()) + return + } + assert.NoError(t, err) + assert.Equal(t, test.expectedMemoryLimit, memoryLimit) + }) + } +} From 9885551ec3ace261652a8bee1f48d4d23a4f514e Mon Sep 17 00:00:00 2001 From: Pritham Marupaka Date: Mon, 13 Nov 2023 17:45:16 -0500 Subject: [PATCH 02/25] Update integration test file --- ...ncher-custom-experimental-processor-aware-heap-pct-flag.yml} | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) rename integration_test/testdata/{launcher-custom-experimental-processor-count.yml => launcher-custom-experimental-processor-aware-heap-pct-flag.yml} (59%) diff --git a/integration_test/testdata/launcher-custom-experimental-processor-count.yml b/integration_test/testdata/launcher-custom-experimental-processor-aware-heap-pct-flag.yml similarity index 59% rename from integration_test/testdata/launcher-custom-experimental-processor-count.yml rename to integration_test/testdata/launcher-custom-experimental-processor-aware-heap-pct-flag.yml index c2457679..f0af476d 100644 --- a/integration_test/testdata/launcher-custom-experimental-processor-count.yml +++ b/integration_test/testdata/launcher-custom-experimental-processor-aware-heap-pct-flag.yml @@ -3,4 +3,4 @@ configVersion: 1 jvmOpts: - '-Xmx1g' experimental: - overrideActiveProcessorCount: true + useProcessorAwareInitialHeapPercentage: true From 2a254673bb981c7af76fbe1050c1e7aa91ba76f2 Mon Sep 17 00:00:00 2001 From: Pritham Marupaka Date: Mon, 13 Nov 2023 17:51:11 -0500 Subject: [PATCH 03/25] add a comment --- launchlib/launcher.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/launchlib/launcher.go b/launchlib/launcher.go index 3ee466e6..83228c95 100644 --- a/launchlib/launcher.go +++ b/launchlib/launcher.go @@ -382,6 +382,8 @@ func isInitialRAMPercentage(arg string) bool { return strings.HasPrefix(arg, "-XX:InitialRAMPercentage=") } +// If the experimental `UseProcessorAwareInitialHeapPercentage` is enabled, compute the heap percentage as 75% of the +// heap minus 3mb per processor, with a minimum value of 50%. func computeInitialHeapPercentage(customConfig *CustomLauncherConfig) (float64, error) { if !customConfig.Experimental.UseProcessorAwareInitialHeapPercentage { return 0.75, nil @@ -396,7 +398,7 @@ func computeInitialHeapPercentage(customConfig *CustomLauncherConfig) (float64, return 0, errors.Wrap(err, "failed to get cgroup memory limit") } var heapLimit = float64(cgroupMemoryLimitInBytes) - var processorOffset = 3 * BytesInMebibyte * float64(cgroupProcessorCount) + var processorAdjustment = 3 * BytesInMebibyte * float64(cgroupProcessorCount) - return max(0.5, (0.75*heapLimit-processorOffset)/heapLimit), nil + return max(0.5, (0.75*heapLimit-processorAdjustment)/heapLimit), nil } From 8047f50d1a7b11d65fa63a1c14445827e700ec3a Mon Sep 17 00:00:00 2001 From: Pritham Marupaka Date: Mon, 13 Nov 2023 18:00:14 -0500 Subject: [PATCH 04/25] Round percentage to 2 decimal places --- launchlib/launcher.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/launchlib/launcher.go b/launchlib/launcher.go index 83228c95..62ea563c 100644 --- a/launchlib/launcher.go +++ b/launchlib/launcher.go @@ -316,8 +316,8 @@ func filterHeapSizeArgs(customConfig *CustomLauncherConfig, args []string) []str if err != nil { initialHeapPercentage = 0.75 } - filtered = append(filtered, fmt.Sprintf("-XX:InitialRAMPercentage=%f", initialHeapPercentage)) - filtered = append(filtered, fmt.Sprintf("-XX:MaxRAMPercentage=%f", initialHeapPercentage)) + filtered = append(filtered, fmt.Sprintf("-XX:InitialRAMPercentage=%.2f", initialHeapPercentage)) + filtered = append(filtered, fmt.Sprintf("-XX:MaxRAMPercentage=%.2f", initialHeapPercentage)) } return filtered } From 694c0c527e2776d0519fe1234bff953d2a3bf411 Mon Sep 17 00:00:00 2001 From: Pritham Marupaka Date: Mon, 13 Nov 2023 18:06:27 -0500 Subject: [PATCH 05/25] Update heap pct calculation --- launchlib/launcher.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/launchlib/launcher.go b/launchlib/launcher.go index 62ea563c..d2a794f3 100644 --- a/launchlib/launcher.go +++ b/launchlib/launcher.go @@ -314,7 +314,7 @@ func filterHeapSizeArgs(customConfig *CustomLauncherConfig, args []string) []str if !hasInitialRAMPercentage && !hasMaxRAMPercentage { initialHeapPercentage, err := computeInitialHeapPercentage(customConfig) if err != nil { - initialHeapPercentage = 0.75 + initialHeapPercentage = 75.0 } filtered = append(filtered, fmt.Sprintf("-XX:InitialRAMPercentage=%.2f", initialHeapPercentage)) filtered = append(filtered, fmt.Sprintf("-XX:MaxRAMPercentage=%.2f", initialHeapPercentage)) @@ -386,7 +386,7 @@ func isInitialRAMPercentage(arg string) bool { // heap minus 3mb per processor, with a minimum value of 50%. func computeInitialHeapPercentage(customConfig *CustomLauncherConfig) (float64, error) { if !customConfig.Experimental.UseProcessorAwareInitialHeapPercentage { - return 0.75, nil + return 75.0, nil } cgroupProcessorCount, err := DefaultCGroupV1ProcessorCounter.ProcessorCount() @@ -400,5 +400,5 @@ func computeInitialHeapPercentage(customConfig *CustomLauncherConfig) (float64, var heapLimit = float64(cgroupMemoryLimitInBytes) var processorAdjustment = 3 * BytesInMebibyte * float64(cgroupProcessorCount) - return max(0.5, (0.75*heapLimit-processorAdjustment)/heapLimit), nil + return max(50.0, (0.75*heapLimit-processorAdjustment)/heapLimit*100.0), nil } From b5311e2999d0c2601cd0d4175c48ed33cbb625c1 Mon Sep 17 00:00:00 2001 From: Pritham Marupaka Date: Mon, 13 Nov 2023 18:08:47 -0500 Subject: [PATCH 06/25] update flag name --- ...er-custom-experimental-processor-aware-heap-pct-flag.yml | 2 +- launchlib/config.go | 2 +- launchlib/launcher.go | 6 +++--- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/integration_test/testdata/launcher-custom-experimental-processor-aware-heap-pct-flag.yml b/integration_test/testdata/launcher-custom-experimental-processor-aware-heap-pct-flag.yml index f0af476d..56f1e36a 100644 --- a/integration_test/testdata/launcher-custom-experimental-processor-aware-heap-pct-flag.yml +++ b/integration_test/testdata/launcher-custom-experimental-processor-aware-heap-pct-flag.yml @@ -3,4 +3,4 @@ configVersion: 1 jvmOpts: - '-Xmx1g' experimental: - useProcessorAwareInitialHeapPercentage: true + useProcessorAwareInitialHeapSize: true diff --git a/launchlib/config.go b/launchlib/config.go index 2eb85b6d..86cf0b94 100644 --- a/launchlib/config.go +++ b/launchlib/config.go @@ -73,7 +73,7 @@ type CustomLauncherConfig struct { type ExperimentalLauncherConfig struct { // Also disables setting the active processor count JVM argument. - UseProcessorAwareInitialHeapPercentage bool `yaml:"useProcessorAwareInitialHeapPercentage"` + UseProcessorAwareInitialHeapSize bool `yaml:"useProcessorAwareInitialHeapSize"` } type PrimaryCustomLauncherConfig struct { diff --git a/launchlib/launcher.go b/launchlib/launcher.go index d2a794f3..8abb0c8e 100644 --- a/launchlib/launcher.go +++ b/launchlib/launcher.go @@ -333,7 +333,7 @@ func ensureActiveProcessorCount(customConfig *CustomLauncherConfig, args []strin filtered = append(filtered, arg) } - if !hasActiveProcessorCount && !customConfig.Experimental.UseProcessorAwareInitialHeapPercentage { + if !hasActiveProcessorCount && !customConfig.Experimental.UseProcessorAwareInitialHeapSize { processorCountArg, err := getActiveProcessorCountArg(logger) if err == nil { filtered = append(filtered, processorCountArg) @@ -382,10 +382,10 @@ func isInitialRAMPercentage(arg string) bool { return strings.HasPrefix(arg, "-XX:InitialRAMPercentage=") } -// If the experimental `UseProcessorAwareInitialHeapPercentage` is enabled, compute the heap percentage as 75% of the +// If the experimental `UseProcessorAwareInitialHeapSize` is set, compute the heap percentage as 75% of the // heap minus 3mb per processor, with a minimum value of 50%. func computeInitialHeapPercentage(customConfig *CustomLauncherConfig) (float64, error) { - if !customConfig.Experimental.UseProcessorAwareInitialHeapPercentage { + if !customConfig.Experimental.UseProcessorAwareInitialHeapSize { return 75.0, nil } From cd02b48f76277a36fb93eeab638c9eb5eb630aa1 Mon Sep 17 00:00:00 2001 From: Pritham Marupaka Date: Mon, 13 Nov 2023 18:20:06 -0500 Subject: [PATCH 07/25] round to 1 decimal place --- launchlib/launcher.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/launchlib/launcher.go b/launchlib/launcher.go index 8abb0c8e..edf69449 100644 --- a/launchlib/launcher.go +++ b/launchlib/launcher.go @@ -316,8 +316,8 @@ func filterHeapSizeArgs(customConfig *CustomLauncherConfig, args []string) []str if err != nil { initialHeapPercentage = 75.0 } - filtered = append(filtered, fmt.Sprintf("-XX:InitialRAMPercentage=%.2f", initialHeapPercentage)) - filtered = append(filtered, fmt.Sprintf("-XX:MaxRAMPercentage=%.2f", initialHeapPercentage)) + filtered = append(filtered, fmt.Sprintf("-XX:InitialRAMPercentage=%.1f", initialHeapPercentage)) + filtered = append(filtered, fmt.Sprintf("-XX:MaxRAMPercentage=%.1f", initialHeapPercentage)) } return filtered } From f2c041a3f130a2906d84fcfa497e92fd915a317a Mon Sep 17 00:00:00 2001 From: svc-changelog Date: Mon, 13 Nov 2023 23:22:45 +0000 Subject: [PATCH 08/25] Add generated changelog entries --- changelog/@unreleased/pr-361.v2.yml | 7 +++++++ 1 file changed, 7 insertions(+) create mode 100644 changelog/@unreleased/pr-361.v2.yml diff --git a/changelog/@unreleased/pr-361.v2.yml b/changelog/@unreleased/pr-361.v2.yml new file mode 100644 index 00000000..f0d638db --- /dev/null +++ b/changelog/@unreleased/pr-361.v2.yml @@ -0,0 +1,7 @@ +type: improvement +improvement: + description: When the experimental UseProcessorAwareInitialHeapSize is set, 1) compute + the heap percentage as 75% of the heap minus 3mb per processor, with a minimum + value of 50%, and 2) don't set the -XX:ActiveProcessorCount JVM option. + links: + - https://github.com/palantir/go-java-launcher/pull/361 From b311ee5186ea8746618c9bdb607973befbdd99b5 Mon Sep 17 00:00:00 2001 From: Pritham Marupaka Date: Tue, 14 Nov 2023 10:47:44 -0500 Subject: [PATCH 09/25] review changes --- README.md | 12 +++++++++++- launchlib/config.go | 3 +-- launchlib/launcher.go | 42 +++++++++++++++++++++++++++--------------- 3 files changed, 39 insertions(+), 18 deletions(-) diff --git a/README.md b/README.md index fc7e215c..0431ec19 100644 --- a/README.md +++ b/README.md @@ -148,7 +148,7 @@ All output from `go-java-launcher` itself, and from the launch of all processes ## Java heap and container support -By default, when starting a java process inside a container (as indicated by the presence of ``CONTAINER`` env +By _default_, when starting a java process inside a container (as indicated by the presence of ``CONTAINER`` env variable): 1. If `-XX:ActiveProcessorCount` is unset, it will be set based on the discovered cgroup configurations and host @@ -162,6 +162,16 @@ variable): This will cause the JVM 11+ to discover the ``MaxRAM`` value using Linux cgroups, and calculate the heap sizes as the specified percentage of ``MaxRAM`` value, e.g. ``max-heap-size = MaxRAM * MaxRamPercentage``. +If the experimental flag `experimentalContainerV2` is set: +1. The `-XX:ActiveProcessorCount` is unset, it will remain unset. +1. Args with prefix``-Xmx|-Xms`` in both static and custom jvm opts will be filtered out. If neither + ``-XX:MaxRAMPercentage=`` nor ``-XX:InitialRAMPercentage=`` prefixes are present in either static or custom jvm opts, + ``-Xmx|-Xms`` will both be set to be 75% of the heap size minus 3mb per processor, with a minimum value of 50% of the + heap. +1. If neither ``-XX:MaxRAMPercentage=`` nor ``-XX:InitialRAMPercentage=`` prefixes are present in either static or + custom jvm opts, both will be set to ``75.0`` (i.e. ``-XX:InitialRAMPercentage=75.0 -XX:MaxRAMPercentage=75.0 `` will + be appended after all the other jvm opts). + ### Overriding default values Developers can override the heap percentage in containers by specifying both ``-XX:MaxRAMPercentage=`` diff --git a/launchlib/config.go b/launchlib/config.go index 86cf0b94..39fa1d20 100644 --- a/launchlib/config.go +++ b/launchlib/config.go @@ -72,8 +72,7 @@ type CustomLauncherConfig struct { } type ExperimentalLauncherConfig struct { - // Also disables setting the active processor count JVM argument. - UseProcessorAwareInitialHeapSize bool `yaml:"useProcessorAwareInitialHeapSize"` + ExperimentalContainerV2 bool `yaml:"experimentalContainerV2"` } type PrimaryCustomLauncherConfig struct { diff --git a/launchlib/launcher.go b/launchlib/launcher.go index edf69449..f4118f3e 100644 --- a/launchlib/launcher.go +++ b/launchlib/launcher.go @@ -312,16 +312,32 @@ func filterHeapSizeArgs(customConfig *CustomLauncherConfig, args []string) []str } if !hasInitialRAMPercentage && !hasMaxRAMPercentage { - initialHeapPercentage, err := computeInitialHeapPercentage(customConfig) - if err != nil { - initialHeapPercentage = 75.0 + if customConfig.Experimental.ExperimentalContainerV2 { + jvmHeapSizeInBytes, err := computeJVMHeapSizeInBytes() + if err != nil { + filtered = setJVMHeapPercentage(filtered) + } else { + filtered = setJVMHeapSize(filtered, jvmHeapSizeInBytes) + } + } else { + filtered = setJVMHeapPercentage(filtered) } - filtered = append(filtered, fmt.Sprintf("-XX:InitialRAMPercentage=%.1f", initialHeapPercentage)) - filtered = append(filtered, fmt.Sprintf("-XX:MaxRAMPercentage=%.1f", initialHeapPercentage)) } return filtered } +func setJVMHeapPercentage(args []string) []string { + args = append(args, "-XX:InitialRAMPercentage=75.0") + args = append(args, "-XX:MaxRAMPercentage=75.0") + return args +} + +func setJVMHeapSize(args []string, heapSizeInBytes uint64) []string { + args = append(args, fmt.Sprintf("-Xms%d", heapSizeInBytes)) + args = append(args, fmt.Sprintf("-Xmx%d", heapSizeInBytes)) + return args +} + func ensureActiveProcessorCount(customConfig *CustomLauncherConfig, args []string, logger io.Writer) []string { filtered := make([]string, 0, len(args)+1) @@ -333,7 +349,7 @@ func ensureActiveProcessorCount(customConfig *CustomLauncherConfig, args []strin filtered = append(filtered, arg) } - if !hasActiveProcessorCount && !customConfig.Experimental.UseProcessorAwareInitialHeapSize { + if !hasActiveProcessorCount && !customConfig.Experimental.ExperimentalContainerV2 { processorCountArg, err := getActiveProcessorCountArg(logger) if err == nil { filtered = append(filtered, processorCountArg) @@ -382,13 +398,9 @@ func isInitialRAMPercentage(arg string) bool { return strings.HasPrefix(arg, "-XX:InitialRAMPercentage=") } -// If the experimental `UseProcessorAwareInitialHeapSize` is set, compute the heap percentage as 75% of the -// heap minus 3mb per processor, with a minimum value of 50%. -func computeInitialHeapPercentage(customConfig *CustomLauncherConfig) (float64, error) { - if !customConfig.Experimental.UseProcessorAwareInitialHeapSize { - return 75.0, nil - } - +// If the experimental `ExperimentalContainerV2` is set, compute the heap size to be 75% of the heap minus 3mb per +// processor, with a minimum value of 50% of the heap. +func computeJVMHeapSizeInBytes() (uint64, error) { cgroupProcessorCount, err := DefaultCGroupV1ProcessorCounter.ProcessorCount() if err != nil { return 0, errors.Wrap(err, "failed to get cgroup processor count") @@ -399,6 +411,6 @@ func computeInitialHeapPercentage(customConfig *CustomLauncherConfig) (float64, } var heapLimit = float64(cgroupMemoryLimitInBytes) var processorAdjustment = 3 * BytesInMebibyte * float64(cgroupProcessorCount) - - return max(50.0, (0.75*heapLimit-processorAdjustment)/heapLimit*100.0), nil + var computedHeapSize = max(0.5*heapLimit, (0.75*heapLimit-processorAdjustment)/heapLimit) + return uint64(computedHeapSize), nil } From 3c7b5727833079dac2d4f28186cf143b1cdaefca Mon Sep 17 00:00:00 2001 From: Pritham Marupaka Date: Tue, 14 Nov 2023 11:08:06 -0500 Subject: [PATCH 10/25] fix computation --- launchlib/launcher.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/launchlib/launcher.go b/launchlib/launcher.go index f4118f3e..793367d4 100644 --- a/launchlib/launcher.go +++ b/launchlib/launcher.go @@ -411,6 +411,6 @@ func computeJVMHeapSizeInBytes() (uint64, error) { } var heapLimit = float64(cgroupMemoryLimitInBytes) var processorAdjustment = 3 * BytesInMebibyte * float64(cgroupProcessorCount) - var computedHeapSize = max(0.5*heapLimit, (0.75*heapLimit-processorAdjustment)/heapLimit) + var computedHeapSize = max(0.5*heapLimit, 0.75*heapLimit-processorAdjustment) return uint64(computedHeapSize), nil } From 52908f8fcc033a0623c288c1b1532bc1ec2374ba Mon Sep 17 00:00:00 2001 From: Pritham Marupaka Date: Tue, 14 Nov 2023 11:55:38 -0500 Subject: [PATCH 11/25] review changes p2 --- launchlib/launcher.go | 77 ++++++++++++++++++++++++++----------------- 1 file changed, 47 insertions(+), 30 deletions(-) diff --git a/launchlib/launcher.go b/launchlib/launcher.go index 793367d4..21fb3b30 100644 --- a/launchlib/launcher.go +++ b/launchlib/launcher.go @@ -21,6 +21,7 @@ import ( "os/exec" "path" "regexp" + "runtime" "strings" "github.com/pkg/errors" @@ -100,7 +101,10 @@ func compileCmdFromConfig( combinedJvmOpts = append(combinedJvmOpts, staticConfig.JavaConfig.JvmOpts...) combinedJvmOpts = append(combinedJvmOpts, customConfig.JvmOpts...) - jvmOpts := createJvmOpts(combinedJvmOpts, customConfig, logger) + jvmOpts, err := createJvmOpts(combinedJvmOpts, customConfig, logger) + if err != nil { + return nil, err + } executable, executableErr = verifyPathIsSafeForExec(path.Join(javaHome, "/bin/java")) if executableErr != nil { @@ -277,12 +281,20 @@ func delim(str string) string { return fmt.Sprintf("%s%s%s", TemplateDelimsOpen, str, TemplateDelimsClose) } -func createJvmOpts(combinedJvmOpts []string, customConfig *CustomLauncherConfig, logger io.WriteCloser) []string { +func createJvmOpts(combinedJvmOpts []string, customConfig *CustomLauncherConfig, logger io.WriteCloser) ([]string, error) { if isEnvVarSet("CONTAINER") && !customConfig.DisableContainerSupport && !hasMaxRAMOverride(combinedJvmOpts) { _, _ = fmt.Fprintln(logger, "Container support enabled") - combinedJvmOpts = filterHeapSizeArgs(customConfig, combinedJvmOpts) + if customConfig.Experimental.ExperimentalContainerV2 { + jvmOptsWithUpdatedHeapSizeArgs, err := filterHeapSizeArgsV2(combinedJvmOpts, logger) + if err != nil { + return nil, err + } + combinedJvmOpts = jvmOptsWithUpdatedHeapSizeArgs + } else { + combinedJvmOpts = filterHeapSizeArgs(combinedJvmOpts) + } combinedJvmOpts = ensureActiveProcessorCount(customConfig, combinedJvmOpts, logger) - return combinedJvmOpts + return combinedJvmOpts, nil } if isEnvVarSet("CONTAINER") { @@ -293,10 +305,10 @@ func createJvmOpts(combinedJvmOpts []string, customConfig *CustomLauncherConfig, } } - return combinedJvmOpts + return combinedJvmOpts, nil } -func filterHeapSizeArgs(customConfig *CustomLauncherConfig, args []string) []string { +func filterHeapSizeArgs(args []string) []string { var filtered []string var hasMaxRAMPercentage, hasInitialRAMPercentage bool for _, arg := range args { @@ -312,30 +324,38 @@ func filterHeapSizeArgs(customConfig *CustomLauncherConfig, args []string) []str } if !hasInitialRAMPercentage && !hasMaxRAMPercentage { - if customConfig.Experimental.ExperimentalContainerV2 { - jvmHeapSizeInBytes, err := computeJVMHeapSizeInBytes() - if err != nil { - filtered = setJVMHeapPercentage(filtered) - } else { - filtered = setJVMHeapSize(filtered, jvmHeapSizeInBytes) - } - } else { - filtered = setJVMHeapPercentage(filtered) - } + filtered = append(filtered, "-XX:InitialRAMPercentage=75.0") + filtered = append(filtered, "-XX:MaxRAMPercentage=75.0") } return filtered } -func setJVMHeapPercentage(args []string) []string { - args = append(args, "-XX:InitialRAMPercentage=75.0") - args = append(args, "-XX:MaxRAMPercentage=75.0") - return args -} +// Used when the experimentalContainerV2 flag is set +func filterHeapSizeArgsV2(args []string, logger io.Writer) ([]string, error) { + var filtered []string + var hasMaxRAMPercentage, hasInitialRAMPercentage bool + for _, arg := range args { + if !isHeapSizeArg(arg) { + filtered = append(filtered, arg) + } + + if isMaxRAMPercentage(arg) { + hasMaxRAMPercentage = true + } else if isInitialRAMPercentage(arg) { + hasInitialRAMPercentage = true + } + } -func setJVMHeapSize(args []string, heapSizeInBytes uint64) []string { - args = append(args, fmt.Sprintf("-Xms%d", heapSizeInBytes)) - args = append(args, fmt.Sprintf("-Xmx%d", heapSizeInBytes)) - return args + if !hasInitialRAMPercentage && !hasMaxRAMPercentage { + jvmHeapSizeInBytes, err := computeJVMHeapSizeInBytes() + if err != nil { + _, _ = fmt.Fprintln(logger, "Failed to compute JVM heap size", err.Error()) + return filtered, err + } + filtered = append(filtered, fmt.Sprintf("-Xms%d", jvmHeapSizeInBytes)) + filtered = append(filtered, fmt.Sprintf("-Xmx%d", jvmHeapSizeInBytes)) + } + return filtered, nil } func ensureActiveProcessorCount(customConfig *CustomLauncherConfig, args []string, logger io.Writer) []string { @@ -401,16 +421,13 @@ func isInitialRAMPercentage(arg string) bool { // If the experimental `ExperimentalContainerV2` is set, compute the heap size to be 75% of the heap minus 3mb per // processor, with a minimum value of 50% of the heap. func computeJVMHeapSizeInBytes() (uint64, error) { - cgroupProcessorCount, err := DefaultCGroupV1ProcessorCounter.ProcessorCount() - if err != nil { - return 0, errors.Wrap(err, "failed to get cgroup processor count") - } + hostProcessorCount := runtime.NumCPU() cgroupMemoryLimitInBytes, err := DefaultMemoryLimit.MemoryLimitInBytes() if err != nil { return 0, errors.Wrap(err, "failed to get cgroup memory limit") } var heapLimit = float64(cgroupMemoryLimitInBytes) - var processorAdjustment = 3 * BytesInMebibyte * float64(cgroupProcessorCount) + var processorAdjustment = 3 * BytesInMebibyte * float64(hostProcessorCount) var computedHeapSize = max(0.5*heapLimit, 0.75*heapLimit-processorAdjustment) return uint64(computedHeapSize), nil } From 7b517e5ba667ce72eccdfb70d82db5bef42dfbcf Mon Sep 17 00:00:00 2001 From: Pritham Marupaka Date: Tue, 14 Nov 2023 11:58:39 -0500 Subject: [PATCH 12/25] wip: adding test --- integration_test/go_java_launcher_integration_test.go | 5 +++++ .../testdata/launcher-custom-experimental-container-v2.yml | 5 +++++ ...er-custom-experimental-processor-aware-heap-pct-flag.yml | 6 ------ 3 files changed, 10 insertions(+), 6 deletions(-) create mode 100644 integration_test/testdata/launcher-custom-experimental-container-v2.yml delete mode 100644 integration_test/testdata/launcher-custom-experimental-processor-aware-heap-pct-flag.yml diff --git a/integration_test/go_java_launcher_integration_test.go b/integration_test/go_java_launcher_integration_test.go index 215b8603..cb4b8a62 100644 --- a/integration_test/go_java_launcher_integration_test.go +++ b/integration_test/go_java_launcher_integration_test.go @@ -69,6 +69,11 @@ func TestMainMethodContainerSupportEnabled(t *testing.T) { launcherCustom: "testdata/launcher-custom-initial-and-max-ram-percentage-override.yml", expectedJVMArgs: "-XX\\:InitialRAMPercentage=79.9 -XX\\:MaxRAMPercentage=80.9 -XX\\:ActiveProcessorCount=2", }, + { + name: "does not set defaults if InitialRAMPercentage and MaxRAMPercentage overrides are present", + launcherCustom: "testdata/launcher-custom-experimental-container-v2.yml", + expectedJVMArgs: "", + }, } { t.Run(tc.name, func(t *testing.T) { testContainerSupportEnabled(t, tc.launcherCustom, tc.expectedJVMArgs) diff --git a/integration_test/testdata/launcher-custom-experimental-container-v2.yml b/integration_test/testdata/launcher-custom-experimental-container-v2.yml new file mode 100644 index 00000000..80d53bce --- /dev/null +++ b/integration_test/testdata/launcher-custom-experimental-container-v2.yml @@ -0,0 +1,5 @@ +configType: java +configVersion: 1 +jvmOpts: {} +experimental: + experimentalContainerV2: true diff --git a/integration_test/testdata/launcher-custom-experimental-processor-aware-heap-pct-flag.yml b/integration_test/testdata/launcher-custom-experimental-processor-aware-heap-pct-flag.yml deleted file mode 100644 index 56f1e36a..00000000 --- a/integration_test/testdata/launcher-custom-experimental-processor-aware-heap-pct-flag.yml +++ /dev/null @@ -1,6 +0,0 @@ -configType: java -configVersion: 1 -jvmOpts: - - '-Xmx1g' -experimental: - useProcessorAwareInitialHeapSize: true From b0b7776e4d7f442d92038abde09fc8e2e76d7768 Mon Sep 17 00:00:00 2001 From: Pritham Marupaka Date: Tue, 14 Nov 2023 12:11:49 -0500 Subject: [PATCH 13/25] wip tests --- .../testdata/launcher-custom-experimental-container-v2.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/integration_test/testdata/launcher-custom-experimental-container-v2.yml b/integration_test/testdata/launcher-custom-experimental-container-v2.yml index 80d53bce..e288a544 100644 --- a/integration_test/testdata/launcher-custom-experimental-container-v2.yml +++ b/integration_test/testdata/launcher-custom-experimental-container-v2.yml @@ -1,5 +1,5 @@ configType: java configVersion: 1 -jvmOpts: {} +jvmOpts: [] experimental: experimentalContainerV2: true From 9a09312b3b1df42f6abd057b948d72c33195eaae Mon Sep 17 00:00:00 2001 From: Pritham Marupaka Date: Tue, 14 Nov 2023 12:41:30 -0500 Subject: [PATCH 14/25] update integration tests --- .../go_java_launcher_integration_test.go | 21 +++++++++++++++++-- ...ntainer-v2-with-initial-ram-percentage.yml | 6 ++++++ ...l-container-v2-with-max-ram-percentage.yml | 6 ++++++ ...rimental-container-v2-with-xms-and-xmx.yml | 7 +++++++ 4 files changed, 38 insertions(+), 2 deletions(-) create mode 100644 integration_test/testdata/launcher-custom-experimental-container-v2-with-initial-ram-percentage.yml create mode 100644 integration_test/testdata/launcher-custom-experimental-container-v2-with-max-ram-percentage.yml create mode 100644 integration_test/testdata/launcher-custom-experimental-container-v2-with-xms-and-xmx.yml diff --git a/integration_test/go_java_launcher_integration_test.go b/integration_test/go_java_launcher_integration_test.go index cb4b8a62..1ada0ae3 100644 --- a/integration_test/go_java_launcher_integration_test.go +++ b/integration_test/go_java_launcher_integration_test.go @@ -70,9 +70,26 @@ func TestMainMethodContainerSupportEnabled(t *testing.T) { expectedJVMArgs: "-XX\\:InitialRAMPercentage=79.9 -XX\\:MaxRAMPercentage=80.9 -XX\\:ActiveProcessorCount=2", }, { - name: "does not set defaults if InitialRAMPercentage and MaxRAMPercentage overrides are present", + name: "using experimentalContainerV2 sets Xms and Xmx and does not set ActiveProcessorCount", + launcherCustom: "testdata/launcher-custom-experimental-container-v2.yml", + expectedJVMArgs: "-Xms3107979264 -Xmx3107979264", + }, + { + name: "using experimentalContainerV2 with InitialRAMPercentage does not set Xms, Xmx, or " + + "ActiveProcessorCount", + launcherCustom: "testdata/launcher-custom-experimental-container-v2-with-initial-ram-percentage.yml", + expectedJVMArgs: "-XX\\\\:InitialRAMPercentage=70.0", + }, + { + name: "using experimentalContainerV2 with MaxRAMPercentage does not set Xms, Xmx, or " + + "ActiveProcessorCount", + launcherCustom: "testdata/launcher-custom-experimental-container-v2-with-max-ram-percentage.yml", + expectedJVMArgs: "-XX\\\\:MaxRAMPercentage=70.0", + }, + { + name: "using experimentalContainerV2 does not use user-provided Xms or Xmx", launcherCustom: "testdata/launcher-custom-experimental-container-v2.yml", - expectedJVMArgs: "", + expectedJVMArgs: "-Xms3107979264 -Xmx3107979264", }, } { t.Run(tc.name, func(t *testing.T) { diff --git a/integration_test/testdata/launcher-custom-experimental-container-v2-with-initial-ram-percentage.yml b/integration_test/testdata/launcher-custom-experimental-container-v2-with-initial-ram-percentage.yml new file mode 100644 index 00000000..d6eeddd1 --- /dev/null +++ b/integration_test/testdata/launcher-custom-experimental-container-v2-with-initial-ram-percentage.yml @@ -0,0 +1,6 @@ +configType: java +configVersion: 1 +jvmOpts: + - '-XX:InitialRAMPercentage=70.0' +experimental: + experimentalContainerV2: true diff --git a/integration_test/testdata/launcher-custom-experimental-container-v2-with-max-ram-percentage.yml b/integration_test/testdata/launcher-custom-experimental-container-v2-with-max-ram-percentage.yml new file mode 100644 index 00000000..f197ae55 --- /dev/null +++ b/integration_test/testdata/launcher-custom-experimental-container-v2-with-max-ram-percentage.yml @@ -0,0 +1,6 @@ +configType: java +configVersion: 1 +jvmOpts: + - '-XX:MaxRAMPercentage=70.0' +experimental: + experimentalContainerV2: true diff --git a/integration_test/testdata/launcher-custom-experimental-container-v2-with-xms-and-xmx.yml b/integration_test/testdata/launcher-custom-experimental-container-v2-with-xms-and-xmx.yml new file mode 100644 index 00000000..fa7fd02a --- /dev/null +++ b/integration_test/testdata/launcher-custom-experimental-container-v2-with-xms-and-xmx.yml @@ -0,0 +1,7 @@ +configType: java +configVersion: 1 +jvmOpts: + - '-Xms1g' + - '-Xmx1g' +experimental: + experimentalContainerV2: true From 2c28dd79e2c1b49d3925c507383604c151e1f3ab Mon Sep 17 00:00:00 2001 From: svc-changelog Date: Tue, 14 Nov 2023 17:43:53 +0000 Subject: [PATCH 15/25] Add generated changelog entries --- changelog/@unreleased/pr-361.v2.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/changelog/@unreleased/pr-361.v2.yml b/changelog/@unreleased/pr-361.v2.yml index f0d638db..380a10ac 100644 --- a/changelog/@unreleased/pr-361.v2.yml +++ b/changelog/@unreleased/pr-361.v2.yml @@ -1,6 +1,6 @@ type: improvement improvement: - description: When the experimental UseProcessorAwareInitialHeapSize is set, 1) compute + description: When the experimental `experimentalContainerV2` is set, 1) compute the heap percentage as 75% of the heap minus 3mb per processor, with a minimum value of 50%, and 2) don't set the -XX:ActiveProcessorCount JVM option. links: From b3649aa1f518a11a3aa261d4f8d648613bf7e839 Mon Sep 17 00:00:00 2001 From: Pritham Marupaka Date: Tue, 14 Nov 2023 13:26:58 -0500 Subject: [PATCH 16/25] integration test: check for existence of keys --- .../go_java_launcher_integration_test.go | 44 +++++++++++-------- 1 file changed, 26 insertions(+), 18 deletions(-) diff --git a/integration_test/go_java_launcher_integration_test.go b/integration_test/go_java_launcher_integration_test.go index 1ada0ae3..ae5d92b3 100644 --- a/integration_test/go_java_launcher_integration_test.go +++ b/integration_test/go_java_launcher_integration_test.go @@ -45,9 +45,10 @@ func TestMainMethod(t *testing.T) { func TestMainMethodContainerSupportEnabled(t *testing.T) { for _, tc := range []struct { - name string - launcherCustom string - expectedJVMArgs string + name string + launcherCustom string + expectedJVMArgs string + expectedJVMArgKeys []string }{ { name: "sets defaults", @@ -70,30 +71,32 @@ func TestMainMethodContainerSupportEnabled(t *testing.T) { expectedJVMArgs: "-XX\\:InitialRAMPercentage=79.9 -XX\\:MaxRAMPercentage=80.9 -XX\\:ActiveProcessorCount=2", }, { - name: "using experimentalContainerV2 sets Xms and Xmx and does not set ActiveProcessorCount", - launcherCustom: "testdata/launcher-custom-experimental-container-v2.yml", - expectedJVMArgs: "-Xms3107979264 -Xmx3107979264", + name: "using experimentalContainerV2 sets Xms and Xmx and does not set ActiveProcessorCount", + launcherCustom: "testdata/launcher-custom-experimental-container-v2.yml", + expectedJVMArgs: "", + expectedJVMArgKeys: []string{"-Xmx", "-Xms"}, }, { name: "using experimentalContainerV2 with InitialRAMPercentage does not set Xms, Xmx, or " + "ActiveProcessorCount", launcherCustom: "testdata/launcher-custom-experimental-container-v2-with-initial-ram-percentage.yml", - expectedJVMArgs: "-XX\\\\:InitialRAMPercentage=70.0", + expectedJVMArgs: "-XX\\:InitialRAMPercentage=70.0", }, { name: "using experimentalContainerV2 with MaxRAMPercentage does not set Xms, Xmx, or " + "ActiveProcessorCount", launcherCustom: "testdata/launcher-custom-experimental-container-v2-with-max-ram-percentage.yml", - expectedJVMArgs: "-XX\\\\:MaxRAMPercentage=70.0", + expectedJVMArgs: "-XX\\:MaxRAMPercentage=70.0", }, { - name: "using experimentalContainerV2 does not use user-provided Xms or Xmx", - launcherCustom: "testdata/launcher-custom-experimental-container-v2.yml", - expectedJVMArgs: "-Xms3107979264 -Xmx3107979264", + name: "using experimentalContainerV2 does not use user-provided Xms or Xmx", + launcherCustom: "testdata/launcher-custom-experimental-container-v2.yml", + expectedJVMArgs: "", + expectedJVMArgKeys: []string{"-Xmx", "-Xms"}, }, } { t.Run(tc.name, func(t *testing.T) { - testContainerSupportEnabled(t, tc.launcherCustom, tc.expectedJVMArgs) + testContainerSupportEnabled(t, tc.launcherCustom, tc.expectedJVMArgs, tc.expectedJVMArgKeys) }) } } @@ -119,7 +122,7 @@ func TestMainMethodContainerSupportDisabled(t *testing.T) { }, } { t.Run(tc.name, func(t *testing.T) { - testInContainer(t, tc.launcherCustom, tc.containerSupportMessage, tc.expectedJVMArgs) + testInContainer(t, tc.launcherCustom, tc.containerSupportMessage, tc.expectedJVMArgs, []string{}) }) } } @@ -141,7 +144,7 @@ func TestMainMethodWithoutCustomConfig(t *testing.T) { } func TestMainMethodContainerWithoutCustomConfig(t *testing.T) { - output := testContainerSupportEnabled(t, "foo", "-XX\\:InitialRAMPercentage=75.0 -XX\\:MaxRAMPercentage=75.0 -XX\\:ActiveProcessorCount=2") + output := testContainerSupportEnabled(t, "foo", "-XX\\:InitialRAMPercentage=75.0 -XX\\:MaxRAMPercentage=75.0 -XX\\:ActiveProcessorCount=2", []string{}) assert.Regexp(t, `Failed to read custom config file, assuming no custom config: foo`, output) } @@ -248,18 +251,23 @@ func runMultiProcess(t *testing.T, cmd *exec.Cmd) map[string]int { return children } -func testContainerSupportEnabled(t *testing.T, launcherCustom string, expectedJvmArgs string) string { - return testInContainer(t, launcherCustom, "Container support enabled", expectedJvmArgs) +func testContainerSupportEnabled(t *testing.T, launcherCustom string, expectedJvmArgs string, expectedJvmArgKeys []string) string { + return testInContainer(t, launcherCustom, "Container support enabled", expectedJvmArgs, expectedJvmArgKeys) } -func testInContainer(t *testing.T, launcherCustom string, containerSupportMessage string, jvmArgs string) string { +func testInContainer(t *testing.T, launcherCustom string, containerSupportMessage string, jvmArgs string, jvmArgKeys []string) string { output, err := runMainWithArgs(t, "testdata/launcher-static.yml", launcherCustom, "CONTAINER=") require.NoError(t, err, "failed: %s", output) // part of expected output from launcher - assert.Regexp(t, `Argument list to executable binary: \[.+/bin/java `+jvmArgs+` -classpath .+/go-java-launcher/integration_test/testdata Main arg1\]`, output) + if jvmArgs != "" { + assert.Regexp(t, `Argument list to executable binary: \[.+/bin/java `+jvmArgs+` -classpath .+/go-java-launcher/integration_test/testdata Main arg1\]`, output) + } // container support detected and running inside container assert.Regexp(t, containerSupportMessage, output) + for _, key := range jvmArgKeys { + assert.Regexp(t, key, output) + } // expected output of Java program assert.Regexp(t, `\nmain method\n`, output) return output From 6890a1a7b9331aa82371fc22b9c3f2be019d7651 Mon Sep 17 00:00:00 2001 From: Pritham Marupaka Date: Tue, 14 Nov 2023 13:59:16 -0500 Subject: [PATCH 17/25] add test for ComputeJVMHeapSizeInBytes --- .../go_java_launcher_integration_test.go | 36 +++++++++++++++++++ launchlib/launcher.go | 19 ++++------ 2 files changed, 43 insertions(+), 12 deletions(-) diff --git a/integration_test/go_java_launcher_integration_test.go b/integration_test/go_java_launcher_integration_test.go index ae5d92b3..c7b19e66 100644 --- a/integration_test/go_java_launcher_integration_test.go +++ b/integration_test/go_java_launcher_integration_test.go @@ -203,6 +203,42 @@ func TestSubProcessesParsedMonitorSignals(t *testing.T) { assert.Len(t, trapped.FindAll(output.Bytes(), -1), 2, "expect two messages that SIGPOLL was caught") } +func TestComputeJVMHeapSize(t *testing.T) { + for _, tc := range []struct { + name string + numHostProcessors int + memoryLimit uint64 + expectedMaxHeapSize uint64 + }{ + { + name: "at least 50% of heap", + numHostProcessors: 1, + memoryLimit: 10 * launchlib.BytesInMebibyte, + // 75% of heap - 3mb*processors = 4.5mb + expectedMaxHeapSize: 5 * launchlib.BytesInMebibyte, + }, + { + name: "computes 75% of heap minus 3mb per processor", + numHostProcessors: 1, + memoryLimit: 12 * launchlib.BytesInMebibyte, + // 75% of heap - 3mb*processors = 6mb + expectedMaxHeapSize: 6 * launchlib.BytesInMebibyte, + }, + { + name: "multiple processors", + numHostProcessors: 3, + memoryLimit: 120 * launchlib.BytesInMebibyte, + // 75% of heap - 3mb*processors = 81mb + expectedMaxHeapSize: 81 * launchlib.BytesInMebibyte, + }, + } { + t.Run(tc.name, func(t *testing.T) { + heapSizeInBytes := launchlib.ComputeJVMHeapSizeInBytes(tc.numHostProcessors, tc.memoryLimit) + assert.Equal(t, heapSizeInBytes, tc.expectedMaxHeapSize) + }) + } +} + func runMainWithArgs(t *testing.T, staticConfigFile, customConfigFile string, env ...string) (string, error) { jdkDir := "jdk" javaHome, err := filepath.Abs(jdkDir) diff --git a/launchlib/launcher.go b/launchlib/launcher.go index 21fb3b30..9b1037d0 100644 --- a/launchlib/launcher.go +++ b/launchlib/launcher.go @@ -347,11 +347,11 @@ func filterHeapSizeArgsV2(args []string, logger io.Writer) ([]string, error) { } if !hasInitialRAMPercentage && !hasMaxRAMPercentage { - jvmHeapSizeInBytes, err := computeJVMHeapSizeInBytes() + cgroupMemoryLimitInBytes, err := DefaultMemoryLimit.MemoryLimitInBytes() if err != nil { - _, _ = fmt.Fprintln(logger, "Failed to compute JVM heap size", err.Error()) - return filtered, err + return filtered, errors.Wrap(err, "failed to get cgroup memory limit") } + jvmHeapSizeInBytes := ComputeJVMHeapSizeInBytes(runtime.NumCPU(), cgroupMemoryLimitInBytes) filtered = append(filtered, fmt.Sprintf("-Xms%d", jvmHeapSizeInBytes)) filtered = append(filtered, fmt.Sprintf("-Xmx%d", jvmHeapSizeInBytes)) } @@ -418,16 +418,11 @@ func isInitialRAMPercentage(arg string) bool { return strings.HasPrefix(arg, "-XX:InitialRAMPercentage=") } -// If the experimental `ExperimentalContainerV2` is set, compute the heap size to be 75% of the heap minus 3mb per -// processor, with a minimum value of 50% of the heap. -func computeJVMHeapSizeInBytes() (uint64, error) { - hostProcessorCount := runtime.NumCPU() - cgroupMemoryLimitInBytes, err := DefaultMemoryLimit.MemoryLimitInBytes() - if err != nil { - return 0, errors.Wrap(err, "failed to get cgroup memory limit") - } +// ComputeJVMHeapSizeInBytes If the experimental `ExperimentalContainerV2` is set, compute the heap size to be 75% of +// the heap minus 3mb per processor, with a minimum value of 50% of the heap. +func ComputeJVMHeapSizeInBytes(hostProcessorCount int, cgroupMemoryLimitInBytes uint64) uint64 { var heapLimit = float64(cgroupMemoryLimitInBytes) var processorAdjustment = 3 * BytesInMebibyte * float64(hostProcessorCount) var computedHeapSize = max(0.5*heapLimit, 0.75*heapLimit-processorAdjustment) - return uint64(computedHeapSize), nil + return uint64(computedHeapSize) } From 89c0f6ec993676c5a349fae76812b2db10512063 Mon Sep 17 00:00:00 2001 From: Pritham Marupaka Date: Tue, 14 Nov 2023 14:20:09 -0500 Subject: [PATCH 18/25] update TestComputeJVMHeapSize test case --- integration_test/go_java_launcher_integration_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/integration_test/go_java_launcher_integration_test.go b/integration_test/go_java_launcher_integration_test.go index c7b19e66..753747e1 100644 --- a/integration_test/go_java_launcher_integration_test.go +++ b/integration_test/go_java_launcher_integration_test.go @@ -220,9 +220,9 @@ func TestComputeJVMHeapSize(t *testing.T) { { name: "computes 75% of heap minus 3mb per processor", numHostProcessors: 1, - memoryLimit: 12 * launchlib.BytesInMebibyte, - // 75% of heap - 3mb*processors = 6mb - expectedMaxHeapSize: 6 * launchlib.BytesInMebibyte, + memoryLimit: 16 * launchlib.BytesInMebibyte, + // 75% of heap - 3mb*processors = 9mb + expectedMaxHeapSize: 9 * launchlib.BytesInMebibyte, }, { name: "multiple processors", From 8489e7a2797f54246beb5b7e209590422c2bd85e Mon Sep 17 00:00:00 2001 From: Carter Kozak Date: Wed, 15 Nov 2023 19:47:35 +0000 Subject: [PATCH 19/25] Autorelease 1.93.0-rc1 [skip ci] From 17043f507df392563dbca6da5dd4daa0a8b02d4e Mon Sep 17 00:00:00 2001 From: Pritham Marupaka Date: Thu, 16 Nov 2023 10:11:43 -0500 Subject: [PATCH 20/25] review changes: rename flag to containerV2 and fallback to setting initial/max ram percentage when memory.limit_in_bytes is not present --- README.md | 2 +- .../go_java_launcher_integration_test.go | 8 ++++---- ...container-v2-with-initial-ram-percentage.yml | 2 +- ...tal-container-v2-with-max-ram-percentage.yml | 2 +- ...perimental-container-v2-with-xms-and-xmx.yml | 2 +- ...auncher-custom-experimental-container-v2.yml | 2 +- launchlib/config.go | 2 +- launchlib/launcher.go | 17 +++++++++-------- 8 files changed, 19 insertions(+), 18 deletions(-) diff --git a/README.md b/README.md index 0431ec19..17a28df0 100644 --- a/README.md +++ b/README.md @@ -162,7 +162,7 @@ variable): This will cause the JVM 11+ to discover the ``MaxRAM`` value using Linux cgroups, and calculate the heap sizes as the specified percentage of ``MaxRAM`` value, e.g. ``max-heap-size = MaxRAM * MaxRamPercentage``. -If the experimental flag `experimentalContainerV2` is set: +If the experimental flag `containerV2` is set: 1. The `-XX:ActiveProcessorCount` is unset, it will remain unset. 1. Args with prefix``-Xmx|-Xms`` in both static and custom jvm opts will be filtered out. If neither ``-XX:MaxRAMPercentage=`` nor ``-XX:InitialRAMPercentage=`` prefixes are present in either static or custom jvm opts, diff --git a/integration_test/go_java_launcher_integration_test.go b/integration_test/go_java_launcher_integration_test.go index 753747e1..9bee79de 100644 --- a/integration_test/go_java_launcher_integration_test.go +++ b/integration_test/go_java_launcher_integration_test.go @@ -71,25 +71,25 @@ func TestMainMethodContainerSupportEnabled(t *testing.T) { expectedJVMArgs: "-XX\\:InitialRAMPercentage=79.9 -XX\\:MaxRAMPercentage=80.9 -XX\\:ActiveProcessorCount=2", }, { - name: "using experimentalContainerV2 sets Xms and Xmx and does not set ActiveProcessorCount", + name: "using containerV2 sets Xms and Xmx and does not set ActiveProcessorCount", launcherCustom: "testdata/launcher-custom-experimental-container-v2.yml", expectedJVMArgs: "", expectedJVMArgKeys: []string{"-Xmx", "-Xms"}, }, { - name: "using experimentalContainerV2 with InitialRAMPercentage does not set Xms, Xmx, or " + + name: "using containerV2 with InitialRAMPercentage does not set Xms, Xmx, or " + "ActiveProcessorCount", launcherCustom: "testdata/launcher-custom-experimental-container-v2-with-initial-ram-percentage.yml", expectedJVMArgs: "-XX\\:InitialRAMPercentage=70.0", }, { - name: "using experimentalContainerV2 with MaxRAMPercentage does not set Xms, Xmx, or " + + name: "using containerV2 with MaxRAMPercentage does not set Xms, Xmx, or " + "ActiveProcessorCount", launcherCustom: "testdata/launcher-custom-experimental-container-v2-with-max-ram-percentage.yml", expectedJVMArgs: "-XX\\:MaxRAMPercentage=70.0", }, { - name: "using experimentalContainerV2 does not use user-provided Xms or Xmx", + name: "using containerV2 does not use user-provided Xms or Xmx", launcherCustom: "testdata/launcher-custom-experimental-container-v2.yml", expectedJVMArgs: "", expectedJVMArgKeys: []string{"-Xmx", "-Xms"}, diff --git a/integration_test/testdata/launcher-custom-experimental-container-v2-with-initial-ram-percentage.yml b/integration_test/testdata/launcher-custom-experimental-container-v2-with-initial-ram-percentage.yml index d6eeddd1..f2ff4319 100644 --- a/integration_test/testdata/launcher-custom-experimental-container-v2-with-initial-ram-percentage.yml +++ b/integration_test/testdata/launcher-custom-experimental-container-v2-with-initial-ram-percentage.yml @@ -3,4 +3,4 @@ configVersion: 1 jvmOpts: - '-XX:InitialRAMPercentage=70.0' experimental: - experimentalContainerV2: true + containerV2: true diff --git a/integration_test/testdata/launcher-custom-experimental-container-v2-with-max-ram-percentage.yml b/integration_test/testdata/launcher-custom-experimental-container-v2-with-max-ram-percentage.yml index f197ae55..e5d905f1 100644 --- a/integration_test/testdata/launcher-custom-experimental-container-v2-with-max-ram-percentage.yml +++ b/integration_test/testdata/launcher-custom-experimental-container-v2-with-max-ram-percentage.yml @@ -3,4 +3,4 @@ configVersion: 1 jvmOpts: - '-XX:MaxRAMPercentage=70.0' experimental: - experimentalContainerV2: true + containerV2: true diff --git a/integration_test/testdata/launcher-custom-experimental-container-v2-with-xms-and-xmx.yml b/integration_test/testdata/launcher-custom-experimental-container-v2-with-xms-and-xmx.yml index fa7fd02a..299a15bf 100644 --- a/integration_test/testdata/launcher-custom-experimental-container-v2-with-xms-and-xmx.yml +++ b/integration_test/testdata/launcher-custom-experimental-container-v2-with-xms-and-xmx.yml @@ -4,4 +4,4 @@ jvmOpts: - '-Xms1g' - '-Xmx1g' experimental: - experimentalContainerV2: true + containerV2: true diff --git a/integration_test/testdata/launcher-custom-experimental-container-v2.yml b/integration_test/testdata/launcher-custom-experimental-container-v2.yml index e288a544..c5d70615 100644 --- a/integration_test/testdata/launcher-custom-experimental-container-v2.yml +++ b/integration_test/testdata/launcher-custom-experimental-container-v2.yml @@ -2,4 +2,4 @@ configType: java configVersion: 1 jvmOpts: [] experimental: - experimentalContainerV2: true + containerV2: true diff --git a/launchlib/config.go b/launchlib/config.go index 39fa1d20..6b1538eb 100644 --- a/launchlib/config.go +++ b/launchlib/config.go @@ -72,7 +72,7 @@ type CustomLauncherConfig struct { } type ExperimentalLauncherConfig struct { - ExperimentalContainerV2 bool `yaml:"experimentalContainerV2"` + ContainerV2 bool `yaml:"containerV2"` } type PrimaryCustomLauncherConfig struct { diff --git a/launchlib/launcher.go b/launchlib/launcher.go index 9b1037d0..214d8e34 100644 --- a/launchlib/launcher.go +++ b/launchlib/launcher.go @@ -284,12 +284,13 @@ func delim(str string) string { func createJvmOpts(combinedJvmOpts []string, customConfig *CustomLauncherConfig, logger io.WriteCloser) ([]string, error) { if isEnvVarSet("CONTAINER") && !customConfig.DisableContainerSupport && !hasMaxRAMOverride(combinedJvmOpts) { _, _ = fmt.Fprintln(logger, "Container support enabled") - if customConfig.Experimental.ExperimentalContainerV2 { - jvmOptsWithUpdatedHeapSizeArgs, err := filterHeapSizeArgsV2(combinedJvmOpts, logger) + if customConfig.Experimental.ContainerV2 { + jvmOptsWithUpdatedHeapSizeArgs, err := filterHeapSizeArgsV2(combinedJvmOpts) if err != nil { - return nil, err + combinedJvmOpts = filterHeapSizeArgs(combinedJvmOpts) + } else { + combinedJvmOpts = jvmOptsWithUpdatedHeapSizeArgs } - combinedJvmOpts = jvmOptsWithUpdatedHeapSizeArgs } else { combinedJvmOpts = filterHeapSizeArgs(combinedJvmOpts) } @@ -330,8 +331,8 @@ func filterHeapSizeArgs(args []string) []string { return filtered } -// Used when the experimentalContainerV2 flag is set -func filterHeapSizeArgsV2(args []string, logger io.Writer) ([]string, error) { +// Used when the containerV2 flag is set +func filterHeapSizeArgsV2(args []string) ([]string, error) { var filtered []string var hasMaxRAMPercentage, hasInitialRAMPercentage bool for _, arg := range args { @@ -369,7 +370,7 @@ func ensureActiveProcessorCount(customConfig *CustomLauncherConfig, args []strin filtered = append(filtered, arg) } - if !hasActiveProcessorCount && !customConfig.Experimental.ExperimentalContainerV2 { + if !hasActiveProcessorCount && !customConfig.Experimental.ContainerV2 { processorCountArg, err := getActiveProcessorCountArg(logger) if err == nil { filtered = append(filtered, processorCountArg) @@ -418,7 +419,7 @@ func isInitialRAMPercentage(arg string) bool { return strings.HasPrefix(arg, "-XX:InitialRAMPercentage=") } -// ComputeJVMHeapSizeInBytes If the experimental `ExperimentalContainerV2` is set, compute the heap size to be 75% of +// ComputeJVMHeapSizeInBytes If the experimental `ContainerV2` is set, compute the heap size to be 75% of // the heap minus 3mb per processor, with a minimum value of 50% of the heap. func ComputeJVMHeapSizeInBytes(hostProcessorCount int, cgroupMemoryLimitInBytes uint64) uint64 { var heapLimit = float64(cgroupMemoryLimitInBytes) From 5d680691ceb7190291c63192487c443e4e567810 Mon Sep 17 00:00:00 2001 From: svc-changelog Date: Thu, 16 Nov 2023 15:12:44 +0000 Subject: [PATCH 21/25] Add generated changelog entries --- changelog/@unreleased/pr-361.v2.yml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/changelog/@unreleased/pr-361.v2.yml b/changelog/@unreleased/pr-361.v2.yml index 380a10ac..c721722f 100644 --- a/changelog/@unreleased/pr-361.v2.yml +++ b/changelog/@unreleased/pr-361.v2.yml @@ -1,7 +1,7 @@ type: improvement improvement: - description: When the experimental `experimentalContainerV2` is set, 1) compute - the heap percentage as 75% of the heap minus 3mb per processor, with a minimum - value of 50%, and 2) don't set the -XX:ActiveProcessorCount JVM option. + description: When the experimental `containerV2` is set, 1) compute the heap percentage + as 75% of the heap minus 3mb per processor, with a minimum value of 50%, and 2) + don't set the -XX:ActiveProcessorCount JVM option. links: - https://github.com/palantir/go-java-launcher/pull/361 From d11eaf4b50a296855477ebe5e077b7a1c6ad514b Mon Sep 17 00:00:00 2001 From: Pritham Marupaka Date: Thu, 16 Nov 2023 10:28:50 -0500 Subject: [PATCH 22/25] review changes: add comment justifying fallback. heapLimit -> memoryLimit --- launchlib/launcher.go | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/launchlib/launcher.go b/launchlib/launcher.go index 214d8e34..727aa25c 100644 --- a/launchlib/launcher.go +++ b/launchlib/launcher.go @@ -287,6 +287,9 @@ func createJvmOpts(combinedJvmOpts []string, customConfig *CustomLauncherConfig, if customConfig.Experimental.ContainerV2 { jvmOptsWithUpdatedHeapSizeArgs, err := filterHeapSizeArgsV2(combinedJvmOpts) if err != nil { + // When we fail to get the memory limit from the cgroups files, fallback to using percentage-based heap + // sizing. While this method doesn't take into account the per-processor memory offset, it is supported + // by all Java platforms. combinedJvmOpts = filterHeapSizeArgs(combinedJvmOpts) } else { combinedJvmOpts = jvmOptsWithUpdatedHeapSizeArgs @@ -422,8 +425,8 @@ func isInitialRAMPercentage(arg string) bool { // ComputeJVMHeapSizeInBytes If the experimental `ContainerV2` is set, compute the heap size to be 75% of // the heap minus 3mb per processor, with a minimum value of 50% of the heap. func ComputeJVMHeapSizeInBytes(hostProcessorCount int, cgroupMemoryLimitInBytes uint64) uint64 { - var heapLimit = float64(cgroupMemoryLimitInBytes) + var memoryLimit = float64(cgroupMemoryLimitInBytes) var processorAdjustment = 3 * BytesInMebibyte * float64(hostProcessorCount) - var computedHeapSize = max(0.5*heapLimit, 0.75*heapLimit-processorAdjustment) + var computedHeapSize = max(0.5*memoryLimit, 0.75*memoryLimit-processorAdjustment) return uint64(computedHeapSize) } From 1a34acf4a2104a0113fcfedb726ba264b496497f Mon Sep 17 00:00:00 2001 From: Pritham Marupaka Date: Thu, 16 Nov 2023 10:36:50 -0500 Subject: [PATCH 23/25] remove check for containerV2 in 'ensureActiveProcessorCount' and move invocation after a flag check --- launchlib/launcher.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/launchlib/launcher.go b/launchlib/launcher.go index 727aa25c..0e4c63a0 100644 --- a/launchlib/launcher.go +++ b/launchlib/launcher.go @@ -296,8 +296,8 @@ func createJvmOpts(combinedJvmOpts []string, customConfig *CustomLauncherConfig, } } else { combinedJvmOpts = filterHeapSizeArgs(combinedJvmOpts) + combinedJvmOpts = ensureActiveProcessorCount(combinedJvmOpts, logger) } - combinedJvmOpts = ensureActiveProcessorCount(customConfig, combinedJvmOpts, logger) return combinedJvmOpts, nil } @@ -362,7 +362,7 @@ func filterHeapSizeArgsV2(args []string) ([]string, error) { return filtered, nil } -func ensureActiveProcessorCount(customConfig *CustomLauncherConfig, args []string, logger io.Writer) []string { +func ensureActiveProcessorCount(args []string, logger io.Writer) []string { filtered := make([]string, 0, len(args)+1) var hasActiveProcessorCount bool @@ -373,7 +373,7 @@ func ensureActiveProcessorCount(customConfig *CustomLauncherConfig, args []strin filtered = append(filtered, arg) } - if !hasActiveProcessorCount && !customConfig.Experimental.ContainerV2 { + if !hasActiveProcessorCount { processorCountArg, err := getActiveProcessorCountArg(logger) if err == nil { filtered = append(filtered, processorCountArg) From 03480a1e243cede0fba54cb5e7043bd170239fd3 Mon Sep 17 00:00:00 2001 From: Pritham Marupaka Date: Thu, 16 Nov 2023 10:51:24 -0500 Subject: [PATCH 24/25] update comment --- launchlib/launcher.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/launchlib/launcher.go b/launchlib/launcher.go index 0e4c63a0..f0d300a0 100644 --- a/launchlib/launcher.go +++ b/launchlib/launcher.go @@ -289,7 +289,7 @@ func createJvmOpts(combinedJvmOpts []string, customConfig *CustomLauncherConfig, if err != nil { // When we fail to get the memory limit from the cgroups files, fallback to using percentage-based heap // sizing. While this method doesn't take into account the per-processor memory offset, it is supported - // by all Java platforms. + // by all platforms using Java. combinedJvmOpts = filterHeapSizeArgs(combinedJvmOpts) } else { combinedJvmOpts = jvmOptsWithUpdatedHeapSizeArgs From ad969c4d6fde82e555d2d92c0b03e9e16d4be469 Mon Sep 17 00:00:00 2001 From: Pritham Marupaka Date: Thu, 16 Nov 2023 12:06:45 -0500 Subject: [PATCH 25/25] review comments: update readme. Don't needlessly propagate a nil error --- README.md | 11 +++++------ launchlib/launcher.go | 11 ++++------- 2 files changed, 9 insertions(+), 13 deletions(-) diff --git a/README.md b/README.md index 17a28df0..d0a366bd 100644 --- a/README.md +++ b/README.md @@ -165,12 +165,11 @@ percentage of ``MaxRAM`` value, e.g. ``max-heap-size = MaxRAM * MaxRamPercentage If the experimental flag `containerV2` is set: 1. The `-XX:ActiveProcessorCount` is unset, it will remain unset. 1. Args with prefix``-Xmx|-Xms`` in both static and custom jvm opts will be filtered out. If neither - ``-XX:MaxRAMPercentage=`` nor ``-XX:InitialRAMPercentage=`` prefixes are present in either static or custom jvm opts, - ``-Xmx|-Xms`` will both be set to be 75% of the heap size minus 3mb per processor, with a minimum value of 50% of the - heap. -1. If neither ``-XX:MaxRAMPercentage=`` nor ``-XX:InitialRAMPercentage=`` prefixes are present in either static or - custom jvm opts, both will be set to ``75.0`` (i.e. ``-XX:InitialRAMPercentage=75.0 -XX:MaxRAMPercentage=75.0 `` will - be appended after all the other jvm opts). + ``-XX:MaxRAMPercentage=`` nor ``-XX:InitialRAMPercentage=`` prefixes are present in either static or custom jvm opts: + - if we can obtain the cgroups memory limit ``-Xmx|-Xms`` will both be set to be 75% of the cgroups memory limit + minus 3mb per processor, with a minimum value of 50% of the heap. + - if we cannot obtain the cgroups memory limit, both RAM percentage values will be set to ``75.0`` (i.e. + ``-XX:InitialRAMPercentage=75.0 -XX:MaxRAMPercentage=75.0 `` will be appended after all the other jvm opts) ### Overriding default values diff --git a/launchlib/launcher.go b/launchlib/launcher.go index f0d300a0..1dde27e2 100644 --- a/launchlib/launcher.go +++ b/launchlib/launcher.go @@ -101,10 +101,7 @@ func compileCmdFromConfig( combinedJvmOpts = append(combinedJvmOpts, staticConfig.JavaConfig.JvmOpts...) combinedJvmOpts = append(combinedJvmOpts, customConfig.JvmOpts...) - jvmOpts, err := createJvmOpts(combinedJvmOpts, customConfig, logger) - if err != nil { - return nil, err - } + jvmOpts := createJvmOpts(combinedJvmOpts, customConfig, logger) executable, executableErr = verifyPathIsSafeForExec(path.Join(javaHome, "/bin/java")) if executableErr != nil { @@ -281,7 +278,7 @@ func delim(str string) string { return fmt.Sprintf("%s%s%s", TemplateDelimsOpen, str, TemplateDelimsClose) } -func createJvmOpts(combinedJvmOpts []string, customConfig *CustomLauncherConfig, logger io.WriteCloser) ([]string, error) { +func createJvmOpts(combinedJvmOpts []string, customConfig *CustomLauncherConfig, logger io.WriteCloser) []string { if isEnvVarSet("CONTAINER") && !customConfig.DisableContainerSupport && !hasMaxRAMOverride(combinedJvmOpts) { _, _ = fmt.Fprintln(logger, "Container support enabled") if customConfig.Experimental.ContainerV2 { @@ -298,7 +295,7 @@ func createJvmOpts(combinedJvmOpts []string, customConfig *CustomLauncherConfig, combinedJvmOpts = filterHeapSizeArgs(combinedJvmOpts) combinedJvmOpts = ensureActiveProcessorCount(combinedJvmOpts, logger) } - return combinedJvmOpts, nil + return combinedJvmOpts } if isEnvVarSet("CONTAINER") { @@ -309,7 +306,7 @@ func createJvmOpts(combinedJvmOpts []string, customConfig *CustomLauncherConfig, } } - return combinedJvmOpts, nil + return combinedJvmOpts } func filterHeapSizeArgs(args []string) []string {