Add the containerV2 flag to compute initial heap percentage using processor count, and disable setting the ActiveProcessorCount JVM option

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=``

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

integration_test/testdata/launcher-custom-experimental-processor-count.yml → integration_test/testdata/launcher-custom-experimental-processor-aware-heap-pct-flag.yml

@@ -3,4 +3,4 @@ configVersion: 1
 jvmOpts:
   - '-Xmx1g'
 experimental:
-  overrideActiveProcessorCount: true
+  useProcessorAwareInitialHeapSize: true

launchlib/config.go

@@ -71,7 +71,9 @@ type CustomLauncherConfig struct {
 	DisableContainerSupport bool                       `yaml:"dangerousDisableContainerSupport"`
 }
 
-type ExperimentalLauncherConfig struct{}
+type ExperimentalLauncherConfig struct {
+	ExperimentalContainerV2 bool `yaml:"experimentalContainerV2"`
+}
 
 type PrimaryCustomLauncherConfig struct {
 	VersionedConfig      `yaml:",inline"`

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,33 @@ func filterHeapSizeArgs(args []string) []string {
 	}
 
 	if !hasInitialRAMPercentage && !hasMaxRAMPercentage {
-		filtered = append(filtered, "-XX:InitialRAMPercentage=75.0")
-		filtered = append(filtered, "-XX:MaxRAMPercentage=75.0")
+		if customConfig.Experimental.ExperimentalContainerV2 {
+			jvmHeapSizeInBytes, err := computeJVMHeapSizeInBytes()
+			if err != nil {
+				filtered = setJVMHeapPercentage(filtered)
+			} else {
+				filtered = setJVMHeapSize(filtered, jvmHeapSizeInBytes)
+			}
+		} else {
+			filtered = setJVMHeapPercentage(filtered)
+		}
 	}
 	return filtered
 }
 
-func ensureActiveProcessorCount(args []string, logger io.Writer) []string {
+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)
 
 	var hasActiveProcessorCount bool
@@ -328,7 +349,7 @@ func ensureActiveProcessorCount(args []string, logger io.Writer) []string {
 		filtered = append(filtered, arg)
 	}
 
-	if !hasActiveProcessorCount {
+	if !hasActiveProcessorCount && !customConfig.Experimental.ExperimentalContainerV2 {
 		processorCountArg, err := getActiveProcessorCountArg(logger)
 		if err == nil {
 			filtered = append(filtered, processorCountArg)
@@ -376,3 +397,20 @@ func isMaxRAMPercentage(arg string) bool {
 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) {
+	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 processorAdjustment = 3 * BytesInMebibyte * float64(cgroupProcessorCount)
+	var computedHeapSize = max(0.5*heapLimit, (0.75*heapLimit-processorAdjustment)/heapLimit)
+	return uint64(computedHeapSize), nil
+}

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
+}

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)
+		})
+	}
+}