From 6cc5001fd8e14f1ab45e7f11cb07dad1b35e1dfa Mon Sep 17 00:00:00 2001 From: Peter Paul Bakker Date: Wed, 29 Apr 2026 08:28:30 +0000 Subject: [PATCH 1/7] Add failing tests and docs for memory calculator regressions (#1257) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Add memory_calculator_issues_test.go with 4 failing tests documenting bugs tracked in cloudfoundry/java-buildpack#1257: * MEMORY_CALCULATOR_STACK_THREADS ignored (LoadConfig() not called) * MEMORY_CALCULATOR_HEADROOM ignored (LoadConfig() not called) * JBP_CONFIG_OPEN_JDK_JRE stack_threads not parsed * JBP_CONFIG_OPEN_JDK_JRE class_count not parsed - Update RUBY_VS_GO_BUILDPACK_COMPARISON.md: memory calculator is no longer 'Identical' — v4 includes user-pinned -Xmx in total memory check (v3 did not), which can cause startup failures in small containers after migration from the Ruby buildpack --- RUBY_VS_GO_BUILDPACK_COMPARISON.md | 39 +++- .../jres/memory_calculator_issues_test.go | 194 ++++++++++++++++++ 2 files changed, 232 insertions(+), 1 deletion(-) create mode 100644 src/java/jres/memory_calculator_issues_test.go diff --git a/RUBY_VS_GO_BUILDPACK_COMPARISON.md b/RUBY_VS_GO_BUILDPACK_COMPARISON.md index aba4a4922..4a6f69bfd 100644 --- a/RUBY_VS_GO_BUILDPACK_COMPARISON.md +++ b/RUBY_VS_GO_BUILDPACK_COMPARISON.md @@ -2191,12 +2191,49 @@ dependencies: | **APM Agents** | ✅ 15 agents | ✅ 14 agents | Missing: Google Stackdriver Debugger (deprecated) | | **Security Providers** | ✅ 6 | ✅ 6 | Identical | | **Database JDBC Injection** | ✅ | ✅ | Identical | -| **Memory Calculator** | ✅ | ✅ | Identical | +| **Memory Calculator** | ✅ v3.13.0 | ✅ v4.2.0 | **Behaviour change** — see below | | **JVMKill Agent** | ✅ | ✅ | Identical | | **Custom JRE Repositories** | ✅ Runtime config | ❌ Requires fork | Breaking change | | **Multi-buildpack** | ⚠️ Via framework | ✅ Native V3 | Go improvement | | **Configuration Overrides** | ✅ | ✅ | Identical (JBP_CONFIG_*) | +#### Memory Calculator Behaviour Change (v3 → v4) + +The memory calculator binary was upgraded from **v3.13.0 to v4.2.0**. Both versions +receive the full `$JAVA_OPTS` string via `--jvm-options`, but they handle a +user-pinned `-Xmx` differently: + +| Behaviour | v3.13.0 (Ruby buildpack) | v4.2.0 (Go buildpack) | +|-----------|--------------------------|------------------------| +| Total memory check | `overhead > total` | `overhead + fixed heap > total` | +| User sets `-Xmx512M`, memory=750M | ✅ passes (overhead ≈ 728M < 750M) | ❌ fails (728M + 512M = 1,240M > 750M) | + +**Impact**: Apps that set `-Xmx` explicitly in `JAVA_OPTS` (or via `JBP_CONFIG_*`) +**and** run in small containers (e.g. 750M) will fail at startup with: + +``` +required memory 1269289K is greater than 750M available for allocation +``` + +**Migration options** (choose one): + +1. **Remove `-Xmx` from `JAVA_OPTS`** — let the calculator size heap automatically. + This is the recommended approach; the calculator will allocate remaining memory + after reserving space for metaspace, code cache, and stack. + +2. **Increase manifest memory** — raise `memory:` to at least 1300M if you need to + keep a fixed `-Xmx512M`. + +3. **Reduce stack threads** — if you cannot change memory or remove `-Xmx`, reduce + thread count via `JBP_CONFIG_OPEN_JDK_JRE`: + ```yaml + env: + JBP_CONFIG_OPEN_JDK_JRE: '{ memory_calculator: { stack_threads: 50 } }' + ``` + Saving 200 threads × 1M = 200M may be enough to fit within the limit. + > **Note**: `JBP_CONFIG_OPEN_JDK_JRE` parsing is a known bug (not yet fixed); + > track progress on the issue tracker. + ### 10.3 Adoption Recommendations **✅ RECOMMENDED for**: diff --git a/src/java/jres/memory_calculator_issues_test.go b/src/java/jres/memory_calculator_issues_test.go new file mode 100644 index 000000000..88e7c5b92 --- /dev/null +++ b/src/java/jres/memory_calculator_issues_test.go @@ -0,0 +1,194 @@ +package jres_test + +import ( + "bytes" + "fmt" + "os" + "path/filepath" + "github.com/cloudfoundry/java-buildpack/src/java/common" + "github.com/cloudfoundry/java-buildpack/src/java/jres" + "github.com/cloudfoundry/libbuildpack" + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +// These tests document and demonstrate known regressions introduced by the +// migration from the Ruby buildpack (memory calculator v3) to the Go buildpack +// (memory calculator v4). Each test is expected to FAIL until the issue is fixed. +// +// Tracked in: https://github.com/cloudfoundry/java-buildpack/issues/1257 + +var _ = Describe("Memory Calculator Issues", func() { + var ( + buildDir string + depsDir string + cacheDir string + ctx *common.Context + jreDir string + ) + + BeforeEach(func() { + var err error + buildDir, err = os.MkdirTemp("", "mc-issues-build") + Expect(err).NotTo(HaveOccurred()) + + depsDir, err = os.MkdirTemp("", "mc-issues-deps") + Expect(err).NotTo(HaveOccurred()) + + cacheDir, err = os.MkdirTemp("", "mc-issues-cache") + Expect(err).NotTo(HaveOccurred()) + + Expect(os.MkdirAll(depsDir+"/0", 0755)).To(Succeed()) + + logBuffer := &bytes.Buffer{} + logger := libbuildpack.NewLogger(logBuffer) + manifest := &libbuildpack.Manifest{} + installer := &libbuildpack.Installer{} + stager := libbuildpack.NewStager([]string{buildDir, cacheDir, depsDir, "0"}, logger, manifest) + command := &libbuildpack.Command{} + + ctx = &common.Context{ + Stager: stager, + Manifest: manifest, + Installer: installer, + Log: logger, + Command: command, + } + + jreDir = filepath.Join(depsDir, "0", "jre") + Expect(os.MkdirAll(filepath.Join(jreDir, "bin"), 0755)).To(Succeed()) + }) + + AfterEach(func() { + os.RemoveAll(buildDir) + os.RemoveAll(depsDir) + os.RemoveAll(cacheDir) + }) + + // fakeBinary writes a placeholder file that detectInstalledCalculator() will find. + // Required for tests that go through Finalize() or GetCalculatorCommand(), + // since both return early if no calculator binary is detected. + fakeBinary := func(version string) { + name := fmt.Sprintf("java-buildpack-memory-calculator-%s", version) + path := filepath.Join(jreDir, "bin", name) + Expect(os.WriteFile(path, []byte("#!/bin/sh\n"), 0755)).To(Succeed()) + } + + // readGeneratedScript returns the content of the generated memory_calculator.sh. + readGeneratedScript := func() string { + path := filepath.Join(depsDir, "0", "bin", "memory_calculator.sh") + data, err := os.ReadFile(path) + Expect(err).NotTo(HaveOccurred()) + return string(data) + } + + // ------------------------------------------------------------------------- + // https://github.com/cloudfoundry/java-buildpack/issues/1257 + // LoadConfig() appears not to be called — MEMORY_CALCULATOR_* env vars silently ignored + // + // LoadConfig() reads MEMORY_CALCULATOR_STACK_THREADS and MEMORY_CALCULATOR_HEADROOM + // but appears not to be invoked during Supply() or Finalize(). Teams cannot + // tune stack_threads or headroom to work around the memory regression. + // + // Expected fix: call LoadConfig() at the start of Supply() (before countClasses()), + // so both stack_threads and headroom overrides are in effect before the script + // is built in Finalize(). + // ------------------------------------------------------------------------- + Describe("#1257: LoadConfig() never called, MEMORY_CALCULATOR_* env vars silently ignored", func() { + It("MEMORY_CALCULATOR_STACK_THREADS env var reduces thread count in generated script", func() { + DeferCleanup(os.Unsetenv, "MEMORY_CALCULATOR_STACK_THREADS") + Expect(os.Setenv("MEMORY_CALCULATOR_STACK_THREADS", "50")).To(Succeed()) + + fakeBinary("4.2.0") + mc := jres.NewMemoryCalculator(ctx, jreDir, "17.0.9", 17) + Expect(mc.Finalize()).To(Succeed()) + + script := readGeneratedScript() + + // This assertion FAILS until LoadConfig() is called at the start of Supply(): + // currently the script will contain --thread-count=250 (the default). + Expect(script).To(ContainSubstring("--thread-count=50"), + "expected --thread-count=50 from MEMORY_CALCULATOR_STACK_THREADS env var, "+ + "but LoadConfig() appears not to be called so the override is silently ignored.\nScript:\n%s", script) + }) + + It("MEMORY_CALCULATOR_HEADROOM env var applies headroom in generated script", func() { + DeferCleanup(os.Unsetenv, "MEMORY_CALCULATOR_HEADROOM") + Expect(os.Setenv("MEMORY_CALCULATOR_HEADROOM", "5")).To(Succeed()) + + fakeBinary("4.2.0") + mc := jres.NewMemoryCalculator(ctx, jreDir, "17.0.9", 17) + Expect(mc.Finalize()).To(Succeed()) + + script := readGeneratedScript() + + // This assertion FAILS until LoadConfig() is called at the start of Supply(): + // currently no --head-room flag appears because headroom defaults to 0. + Expect(script).To(ContainSubstring("--head-room=5"), + "expected --head-room=5 from MEMORY_CALCULATOR_HEADROOM env var, "+ + "but LoadConfig() appears not to be called.\nScript:\n%s", script) + }) + }) + + // ------------------------------------------------------------------------- + // https://github.com/cloudfoundry/java-buildpack/issues/1257 + // JBP_CONFIG_OPEN_JDK_JRE env var is not parsed + // + // The CF convention for tuning the memory calculator is: + // JBP_CONFIG_OPEN_JDK_JRE: '{ memory_calculator: { stack_threads: 50 } }' + // + // Two bugs appear to prevent this from working: + // a) LoadConfig() appears not to be called during Supply() or Finalize() + // b) LoadConfig() reads MEMORY_CALCULATOR_STACK_THREADS instead of + // parsing JBP_CONFIG_OPEN_JDK_JRE + // + // Note: class_count override must be loaded before countClasses() in Supply(); + // stack_threads must be loaded before buildCalculatorCommand() in Finalize(). + // Therefore LoadConfig() should be called at the start of Supply(). + // + // Reducing stack_threads from 250 → 50 saves 200M of stack, which is the + // primary mitigation available to teams hitting the 750M regression. + // ------------------------------------------------------------------------- + Describe("#1257: JBP_CONFIG_OPEN_JDK_JRE is not parsed, stack_threads override silently ignored", func() { + It("stack_threads set via JBP_CONFIG_OPEN_JDK_JRE is reflected in generated script", func() { + DeferCleanup(os.Unsetenv, "JBP_CONFIG_OPEN_JDK_JRE") + Expect(os.Setenv("JBP_CONFIG_OPEN_JDK_JRE", + "{ memory_calculator: { stack_threads: 50 } }")).To(Succeed()) + + fakeBinary("4.2.0") + mc := jres.NewMemoryCalculator(ctx, jreDir, "17.0.9", 17) + Expect(mc.Finalize()).To(Succeed()) + + script := readGeneratedScript() + + // This assertion FAILS until JBP_CONFIG_OPEN_JDK_JRE is parsed: + // currently the script will contain --thread-count=250 (the default). + // Fix: parse JBP_CONFIG_OPEN_JDK_JRE in LoadConfig() and call it at + // the start of Supply(). With 50 threads: stack = 50M instead of 250M + // → saves 200M, making 750M containers viable again. + Expect(script).To(ContainSubstring("--thread-count=50"), + "expected --thread-count=50 from JBP_CONFIG_OPEN_JDK_JRE but got default 250.\n"+ + "Fix: parse JBP_CONFIG_OPEN_JDK_JRE in LoadConfig() and call it at start of Supply().\nScript:\n%s", + script) + }) + + It("class_count set via JBP_CONFIG_OPEN_JDK_JRE overrides calculated count", func() { + DeferCleanup(os.Unsetenv, "JBP_CONFIG_OPEN_JDK_JRE") + Expect(os.Setenv("JBP_CONFIG_OPEN_JDK_JRE", + "{ memory_calculator: { class_count: 3500 } }")).To(Succeed()) + + fakeBinary("4.2.0") + mc := jres.NewMemoryCalculator(ctx, jreDir, "17.0.9", 17) + Expect(mc.Finalize()).To(Succeed()) + + script := readGeneratedScript() + + // This assertion FAILS until JBP_CONFIG_OPEN_JDK_JRE is parsed. + // class_count must be loaded before countClasses() runs in Supply(). + // class_count=3500 keeps metaspace at ~34M instead of ~233M. + Expect(script).To(ContainSubstring("--loaded-class-count=3500"), + "expected --loaded-class-count=3500 from JBP_CONFIG_OPEN_JDK_JRE.\nScript:\n%s", + script) + }) + }) +}) From ce98f0930c5ea57e6423b44ced3352295c985d07 Mon Sep 17 00:00:00 2001 From: Peter Paul Bakker Date: Wed, 29 Apr 2026 09:23:52 +0000 Subject: [PATCH 2/7] Fix #1257: parse JBP_CONFIG_OPEN_JDK_JRE and call LoadConfig() in Supply/Finalize - Add openJDKJREConfig / memoryCalculatorConfig structs for YAML parsing - Rewrite LoadConfig() to parse JBP_CONFIG_OPEN_JDK_JRE using common.YamlHandler (standard CF config pattern); MEMORY_CALCULATOR_* env vars remain supported as a more specific override - Guard with configLoaded flag to prevent double-loading when both Supply() and Finalize() are called on the same instance - Call LoadConfig() at start of Supply() so class_count override takes effect before countClasses() runs; skip countClasses() if already set - Call LoadConfig() in Finalize() so stackThreads/headroom/classCount are applied when building the calculator command (separate process) Fixes: MEMORY_CALCULATOR_STACK_THREADS/HEADROOM silently ignored Fixes: JBP_CONFIG_OPEN_JDK_JRE stack_threads/class_count silently ignored --- src/java/jres/memory_calculator.go | 60 ++++++++++++++++++++++++------ 1 file changed, 49 insertions(+), 11 deletions(-) diff --git a/src/java/jres/memory_calculator.go b/src/java/jres/memory_calculator.go index b1777ce62..ed9421d84 100644 --- a/src/java/jres/memory_calculator.go +++ b/src/java/jres/memory_calculator.go @@ -24,6 +24,7 @@ type MemoryCalculator struct { classCount int stackThreads int headroom int + configLoaded bool } // NewMemoryCalculator creates a new memory calculator @@ -54,6 +55,8 @@ func (m *MemoryCalculator) Supply() error { m.version = dep.Version m.ctx.Log.Info("Installing Memory Calculator (%s)", m.version) + m.LoadConfig() + // Create bin directory binDir := filepath.Join(m.jreDir, "bin") if err := os.MkdirAll(binDir, 0755); err != nil { @@ -107,10 +110,11 @@ func (m *MemoryCalculator) Supply() error { m.calculatorPath = finalPath - // Count classes in the application - if err := m.countClasses(); err != nil { - m.ctx.Log.Warning("Failed to count classes: %s (using default)", err.Error()) - m.classCount = 0 // Will be calculated as 35% of actual later + // Count classes in the application, unless overridden by config + if m.classCount == 0 { + if err := m.countClasses(); err != nil { + m.ctx.Log.Warning("Failed to count classes: %s (using default)", err.Error()) + } } m.ctx.Log.Info("Memory Calculator installed: Loaded Classes: %d, Threads: %d", @@ -152,6 +156,8 @@ func (m *MemoryCalculator) detectInstalledCalculator() { // Finalize configures the memory calculator in the startup command func (m *MemoryCalculator) Finalize() error { + m.LoadConfig() + // If calculatorPath not set, try to detect it from previous installation if m.calculatorPath == "" { m.detectInstalledCalculator() @@ -364,15 +370,47 @@ func (m *MemoryCalculator) convertToRuntimePath(stagingPath string) string { return fmt.Sprintf("/home/vcap/deps/%s/jre/bin/%s", depsIdx, filename) } -// LoadConfig loads memory calculator configuration from environment/config -func (m *MemoryCalculator) LoadConfig() { - // Check for environment overrides - // JBP_CONFIG_OPEN_JDK_JRE='{memory_calculator: {stack_threads: 300}}' +// openJDKJREConfig mirrors the memory_calculator section of JBP_CONFIG_OPEN_JDK_JRE. +type openJDKJREConfig struct { + MemoryCalculator memoryCalculatorConfig `yaml:"memory_calculator"` +} + +type memoryCalculatorConfig struct { + StackThreads int `yaml:"stack_threads"` + ClassCount int `yaml:"class_count"` + Headroom int `yaml:"headroom"` +} - // For now, using defaults - // In production, we'd parse JSON from environment variables +// LoadConfig loads memory calculator configuration from JBP_CONFIG_OPEN_JDK_JRE +// (standard CF format) and falls back to MEMORY_CALCULATOR_* env vars. +// Must be called at the start of Supply(), before countClasses(). +func (m *MemoryCalculator) LoadConfig() { + if m.configLoaded { + return + } + m.configLoaded = true + if config := os.Getenv("JBP_CONFIG_OPEN_JDK_JRE"); config != "" { + cfg := openJDKJREConfig{} + yamlHandler := common.YamlHandler{} + if err := yamlHandler.ValidateFields([]byte(config), &cfg); err != nil { + m.ctx.Log.Warning("Unknown fields in JBP_CONFIG_OPEN_JDK_JRE: %s", err.Error()) + } + if err := yamlHandler.Unmarshal([]byte(config), &cfg); err != nil { + m.ctx.Log.Warning("Failed to parse JBP_CONFIG_OPEN_JDK_JRE: %s", err.Error()) + } else { + mc := cfg.MemoryCalculator + if mc.StackThreads > 0 { + m.stackThreads = mc.StackThreads + } + if mc.ClassCount > 0 { + m.classCount = mc.ClassCount + } + if mc.Headroom > 0 { + m.headroom = mc.Headroom + } + } + } - // Check specific environment variables if val := os.Getenv("MEMORY_CALCULATOR_STACK_THREADS"); val != "" { if threads, err := strconv.Atoi(val); err == nil { m.stackThreads = threads From 0c9c8f1c50c7371830938293199e4540690a0b37 Mon Sep 17 00:00:00 2001 From: Peter Paul Bakker Date: Wed, 29 Apr 2026 09:53:41 +0000 Subject: [PATCH 3/7] fix: validate memory_calculator sub-section only in JBP_CONFIG_OPEN_JDK_JRE Avoids spurious WARNING when jre: or other valid top-level keys are present in JBP_CONFIG_OPEN_JDK_JRE alongside memory_calculator:. Previously ValidateFields was run on the full config struct, which caused warnings for unknown fields like 'jre' that are handled elsewhere in the buildpack. Now only the memory_calculator sub-section is validated, so typos in memory_calculator fields (e.g. stack_thread instead of stack_threads) still produce a WARNING while legitimate top-level keys are silently ignored. Added two tests: - no WARNING when jre: is present alongside memory_calculator: - WARNING when memory_calculator: contains an unknown/typo'd field --- src/java/jres/memory_calculator.go | 18 +++++-- .../jres/memory_calculator_issues_test.go | 51 ++++++++++++++++--- 2 files changed, 60 insertions(+), 9 deletions(-) diff --git a/src/java/jres/memory_calculator.go b/src/java/jres/memory_calculator.go index ed9421d84..353b2c760 100644 --- a/src/java/jres/memory_calculator.go +++ b/src/java/jres/memory_calculator.go @@ -390,11 +390,23 @@ func (m *MemoryCalculator) LoadConfig() { } m.configLoaded = true if config := os.Getenv("JBP_CONFIG_OPEN_JDK_JRE"); config != "" { - cfg := openJDKJREConfig{} yamlHandler := common.YamlHandler{} - if err := yamlHandler.ValidateFields([]byte(config), &cfg); err != nil { - m.ctx.Log.Warning("Unknown fields in JBP_CONFIG_OPEN_JDK_JRE: %s", err.Error()) + + // Extract raw memory_calculator sub-section to validate its fields separately, + // so unknown top-level keys (e.g. jre:) are silently ignored while typos + // inside memory_calculator: are warned about. + rawCfg := struct { + MC interface{} `yaml:"memory_calculator"` + }{} + if err := yamlHandler.Unmarshal([]byte(config), &rawCfg); err == nil && rawCfg.MC != nil { + if mcBytes, err := yamlHandler.Marshal(rawCfg.MC); err == nil { + if err := yamlHandler.ValidateFields(mcBytes, &memoryCalculatorConfig{}); err != nil { + m.ctx.Log.Warning("Unknown fields in JBP_CONFIG_OPEN_JDK_JRE memory_calculator: %s", err.Error()) + } + } } + + cfg := openJDKJREConfig{} if err := yamlHandler.Unmarshal([]byte(config), &cfg); err != nil { m.ctx.Log.Warning("Failed to parse JBP_CONFIG_OPEN_JDK_JRE: %s", err.Error()) } else { diff --git a/src/java/jres/memory_calculator_issues_test.go b/src/java/jres/memory_calculator_issues_test.go index 88e7c5b92..b4621f961 100644 --- a/src/java/jres/memory_calculator_issues_test.go +++ b/src/java/jres/memory_calculator_issues_test.go @@ -20,11 +20,12 @@ import ( var _ = Describe("Memory Calculator Issues", func() { var ( - buildDir string - depsDir string - cacheDir string - ctx *common.Context - jreDir string + buildDir string + depsDir string + cacheDir string + ctx *common.Context + jreDir string + logBuffer *bytes.Buffer ) BeforeEach(func() { @@ -40,7 +41,7 @@ var _ = Describe("Memory Calculator Issues", func() { Expect(os.MkdirAll(depsDir+"/0", 0755)).To(Succeed()) - logBuffer := &bytes.Buffer{} + logBuffer = &bytes.Buffer{} logger := libbuildpack.NewLogger(logBuffer) manifest := &libbuildpack.Manifest{} installer := &libbuildpack.Installer{} @@ -191,4 +192,42 @@ var _ = Describe("Memory Calculator Issues", func() { script) }) }) + + // ------------------------------------------------------------------------- + // JBP_CONFIG_OPEN_JDK_JRE field validation: + // - Unknown top-level fields (e.g. jre:) must be silently ignored + // - Unknown fields inside memory_calculator: must produce a WARNING (typo guard) + // ------------------------------------------------------------------------- + Describe("JBP_CONFIG_OPEN_JDK_JRE field validation", func() { + It("applies memory_calculator settings and logs no WARNING when jre: is also present", func() { + DeferCleanup(os.Unsetenv, "JBP_CONFIG_OPEN_JDK_JRE") + Expect(os.Setenv("JBP_CONFIG_OPEN_JDK_JRE", + `{ jre: { version: "25.+" }, memory_calculator: { headroom: 5, stack_threads: 50 } }`)).To(Succeed()) + + fakeBinary("4.2.0") + mc := jres.NewMemoryCalculator(ctx, jreDir, "17.0.9", 17) + Expect(mc.Finalize()).To(Succeed()) + + script := readGeneratedScript() + Expect(script).To(ContainSubstring("--thread-count=50")) + Expect(script).To(ContainSubstring("--head-room=5")) + Expect(logBuffer.String()).NotTo(ContainSubstring("WARNING"), + "unexpected WARNING in log output:\n%s", logBuffer.String()) + }) + + It("logs a WARNING when memory_calculator contains an unknown field (e.g. typo)", func() { + DeferCleanup(os.Unsetenv, "JBP_CONFIG_OPEN_JDK_JRE") + Expect(os.Setenv("JBP_CONFIG_OPEN_JDK_JRE", + `{ memory_calculator: { stack_thread: 50 } }`)).To(Succeed()) // typo: missing 's' + + fakeBinary("4.2.0") + mc := jres.NewMemoryCalculator(ctx, jreDir, "17.0.9", 17) + Expect(mc.Finalize()).To(Succeed()) + + Expect(logBuffer.String()).To(ContainSubstring("WARNING"), + "expected WARNING for unknown field 'stack_thread' in memory_calculator") + Expect(logBuffer.String()).To(ContainSubstring("stack_thread"), + "WARNING should mention the unknown field name") + }) + }) }) From 67cbb82b02c0591fe50cc8091915386b0c51bcfb Mon Sep 17 00:00:00 2001 From: Peter Paul Bakker Date: Wed, 29 Apr 2026 10:25:48 +0000 Subject: [PATCH 4/7] feat: improve Memory Calculator install log with source indicators Log message now shows whether each value was user-supplied or not: - Loaded Classes: 23826 (auto-detected) vs 23826 when user-set - Threads: 250 (default) vs 50 when user-set - Headroom: 0% (default) vs 5% when user-set Adds classCountUserSet, stackThreadsUserSet, headroomUserSet flags to MemoryCalculator set during LoadConfig() when a value comes from JBP_CONFIG_OPEN_JDK_JRE or MEMORY_CALCULATOR_* env vars. --- src/java/jres/memory_calculator.go | 41 +++++++++++++++++++++++++----- 1 file changed, 35 insertions(+), 6 deletions(-) diff --git a/src/java/jres/memory_calculator.go b/src/java/jres/memory_calculator.go index 353b2c760..b08b03cfc 100644 --- a/src/java/jres/memory_calculator.go +++ b/src/java/jres/memory_calculator.go @@ -21,10 +21,13 @@ type MemoryCalculator struct { javaMajorVersion int calculatorPath string version string - classCount int - stackThreads int - headroom int - configLoaded bool + classCount int + stackThreads int + headroom int + configLoaded bool + classCountUserSet bool + stackThreadsUserSet bool + headroomUserSet bool } // NewMemoryCalculator creates a new memory calculator @@ -117,8 +120,8 @@ func (m *MemoryCalculator) Supply() error { } } - m.ctx.Log.Info("Memory Calculator installed: Loaded Classes: %d, Threads: %d", - m.classCount, m.stackThreads) + m.ctx.Log.Info("Memory Calculator installed: Loaded Classes: %s, Threads: %s, Headroom: %s", + m.classCountDisplay(), m.stackThreadsDisplay(), m.headroomDisplay()) // Clean up temp directory os.RemoveAll(tempDir) @@ -413,12 +416,15 @@ func (m *MemoryCalculator) LoadConfig() { mc := cfg.MemoryCalculator if mc.StackThreads > 0 { m.stackThreads = mc.StackThreads + m.stackThreadsUserSet = true } if mc.ClassCount > 0 { m.classCount = mc.ClassCount + m.classCountUserSet = true } if mc.Headroom > 0 { m.headroom = mc.Headroom + m.headroomUserSet = true } } } @@ -426,12 +432,14 @@ func (m *MemoryCalculator) LoadConfig() { if val := os.Getenv("MEMORY_CALCULATOR_STACK_THREADS"); val != "" { if threads, err := strconv.Atoi(val); err == nil { m.stackThreads = threads + m.stackThreadsUserSet = true } } if val := os.Getenv("MEMORY_CALCULATOR_HEADROOM"); val != "" { if headroom, err := strconv.Atoi(val); err == nil { m.headroom = headroom + m.headroomUserSet = true } } } @@ -445,6 +453,27 @@ func copyFile(src, dst string) error { return os.WriteFile(dst, data, 0755) } +func (m *MemoryCalculator) classCountDisplay() string { + if m.classCountUserSet { + return fmt.Sprintf("%d", m.classCount) + } + return fmt.Sprintf("%d (auto-detected)", m.classCount) +} + +func (m *MemoryCalculator) stackThreadsDisplay() string { + if m.stackThreadsUserSet { + return fmt.Sprintf("%d", m.stackThreads) + } + return fmt.Sprintf("%d (default)", m.stackThreads) +} + +func (m *MemoryCalculator) headroomDisplay() string { + if m.headroomUserSet { + return fmt.Sprintf("%d%%", m.headroom) + } + return fmt.Sprintf("%d%% (default)", m.headroom) +} + // RunMemoryCalculator runs the memory calculator and returns the calculated JAVA_OPTS // This is primarily for testing func (m *MemoryCalculator) RunMemoryCalculator(memoryLimit string) (string, error) { From 8e884ea93fca3a41b2f695fbd36f644e5b90dea4 Mon Sep 17 00:00:00 2001 From: Peter Paul Bakker Date: Wed, 29 Apr 2026 10:50:20 +0000 Subject: [PATCH 5/7] docs: improve memory calculator v3 vs v4 migration section MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Fix table: show that v3 squeezes non-heap to fit, v4 calculates non-heap independently and correctly includes fixed -Xmx in check - Remove incorrect '(or via JBP_CONFIG_*)' — -Xmx can only be set in JAVA_OPTS, not via JBP_CONFIG_* - Remove stale 'known bug' note about JBP_CONFIG_OPEN_JDK_JRE parsing (now fixed) - Add explanation that v4 behaviour is correct: v3 silently undersized non-heap (thread stacks, metaspace, code cache) when -Xmx was set - Reorder migration options: lower stack_threads moved to option 2 - Add virtual threads note: carrier threads only consume native stack memory; virtual thread continuations live on the heap — set stack_threads to carrier thread count (~CPU cores) for Java 21+ apps - Use realistic memory estimates (~900-1000M for -Xmx512M app) --- RUBY_VS_GO_BUILDPACK_COMPARISON.md | 39 +++++++++++++++++++++--------- 1 file changed, 27 insertions(+), 12 deletions(-) diff --git a/RUBY_VS_GO_BUILDPACK_COMPARISON.md b/RUBY_VS_GO_BUILDPACK_COMPARISON.md index 4a6f69bfd..a8511bce5 100644 --- a/RUBY_VS_GO_BUILDPACK_COMPARISON.md +++ b/RUBY_VS_GO_BUILDPACK_COMPARISON.md @@ -2205,34 +2205,49 @@ user-pinned `-Xmx` differently: | Behaviour | v3.13.0 (Ruby buildpack) | v4.2.0 (Go buildpack) | |-----------|--------------------------|------------------------| -| Total memory check | `overhead > total` | `overhead + fixed heap > total` | -| User sets `-Xmx512M`, memory=750M | ✅ passes (overhead ≈ 728M < 750M) | ❌ fails (728M + 512M = 1,240M > 750M) | +| Total memory check | `non-heap > total` | `non-heap + fixed heap > total` | +| Non-heap calculation when `-Xmx` is set | Squeezed to `total - Xmx` | Calculated independently (threads + metaspace + code cache) | +| User sets `-Xmx512M`, container=750M | ✅ passes — non-heap squeezes into 238M | ❌ fails — non-heap ~400M + 512M heap = ~912M > 750M | -**Impact**: Apps that set `-Xmx` explicitly in `JAVA_OPTS` (or via `JBP_CONFIG_*`) -**and** run in small containers (e.g. 750M) will fail at startup with: +> **Note**: v4 correctly accounts for the full JVM memory footprint. When `-Xmx` is +> explicitly set, it is included in the total memory check — this is the right behaviour +> since the JVM will actually claim that heap in addition to native non-heap memory +> (thread stacks, metaspace, code cache). v3 silently squeezed non-heap into whatever +> remained after `-Xmx`, which could leave thread stacks and metaspace dangerously +> undersized at runtime. + +**Impact**: Apps that set `-Xmx` explicitly in `JAVA_OPTS` +**and** run in containers that cannot accommodate heap + non-heap will fail at startup with: ``` required memory 1269289K is greater than 750M available for allocation ``` +The required memory is: `-Xmx` + thread stacks + metaspace + code cache. +With defaults (250 threads × ~1M = 250M stacks, ~100M metaspace, ~50M code cache): +a `-Xmx512M` app needs roughly **900–1000M** total. + **Migration options** (choose one): 1. **Remove `-Xmx` from `JAVA_OPTS`** — let the calculator size heap automatically. This is the recommended approach; the calculator will allocate remaining memory after reserving space for metaspace, code cache, and stack. -2. **Increase manifest memory** — raise `memory:` to at least 1300M if you need to - keep a fixed `-Xmx512M`. - -3. **Reduce stack threads** — if you cannot change memory or remove `-Xmx`, reduce - thread count via `JBP_CONFIG_OPEN_JDK_JRE`: +2. **Lower `stack_threads`** — the default 250 platform threads claim ~250M of native + memory. Reducing this is often the easiest way to fit within the container: ```yaml env: JBP_CONFIG_OPEN_JDK_JRE: '{ memory_calculator: { stack_threads: 50 } }' ``` - Saving 200 threads × 1M = 200M may be enough to fit within the limit. - > **Note**: `JBP_CONFIG_OPEN_JDK_JRE` parsing is a known bug (not yet fixed); - > track progress on the issue tracker. + For apps using **virtual threads** (Java 21+, Spring Boot 3.2+), set this to the + carrier thread count (~CPU cores, typically 4–16). Virtual thread stacks are stored + on the heap as continuation objects, not as native stack memory, so only carrier + threads consume native stack memory. Note: with virtual threads you may want to + keep or increase `-Xmx` to accommodate heap-resident continuations. + +3. **Increase manifest memory** — raise `memory:` in `manifest.yml` to accommodate + the full JVM footprint (`-Xmx` + non-heap). With `-Xmx512M` and default thread + count, plan for ~900–1000M. ### 10.3 Adoption Recommendations From 81a214056f4edf4b0f5dab1939d809b55403f23e Mon Sep 17 00:00:00 2001 From: Peter Paul Bakker Date: Wed, 29 Apr 2026 11:38:54 +0000 Subject: [PATCH 6/7] docs: tighten memory calculator v3 vs v4 migration section - Clarify the v3 vs v4 difference only applies when explicit -Xmx is set - Note that without explicit -Xmx the startup failure does not occur - Note v3 claimed less memory by squeezing non-heap alongside fixed -Xmx - Reorder migration options: lower stack_threads first, with caveat to only do so when app actually uses fewer than 250 threads - Clarify that removing -Xmx still likely requires increasing manifest memory - Fix memory figure to at least 1300M based on actual observed error --- RUBY_VS_GO_BUILDPACK_COMPARISON.md | 50 ++++++++---------------------- 1 file changed, 13 insertions(+), 37 deletions(-) diff --git a/RUBY_VS_GO_BUILDPACK_COMPARISON.md b/RUBY_VS_GO_BUILDPACK_COMPARISON.md index a8511bce5..6fe683e55 100644 --- a/RUBY_VS_GO_BUILDPACK_COMPARISON.md +++ b/RUBY_VS_GO_BUILDPACK_COMPARISON.md @@ -2199,55 +2199,31 @@ dependencies: #### Memory Calculator Behaviour Change (v3 → v4) -The memory calculator binary was upgraded from **v3.13.0 to v4.2.0**. Both versions -receive the full `$JAVA_OPTS` string via `--jvm-options`, but they handle a -user-pinned `-Xmx` differently: +The memory calculator was upgraded from **v3.13.0 to v4.2.0**. The difference only affects apps with an **explicit `-Xmx`** in `JAVA_OPTS` (setting `-Xmx` explicitly in containerised environments is generally considered bad practice — the calculator sizes heap better automatically). How a pinned `-Xmx` is handled: -| Behaviour | v3.13.0 (Ruby buildpack) | v4.2.0 (Go buildpack) | -|-----------|--------------------------|------------------------| -| Total memory check | `non-heap > total` | `non-heap + fixed heap > total` | -| Non-heap calculation when `-Xmx` is set | Squeezed to `total - Xmx` | Calculated independently (threads + metaspace + code cache) | -| User sets `-Xmx512M`, container=750M | ✅ passes — non-heap squeezes into 238M | ❌ fails — non-heap ~400M + 512M heap = ~912M > 750M | +| | v3.13.0 (Ruby) | v4.2.0 (Go) | +|--|----------------|-------------| +| Memory check | `non-heap > total` | `non-heap + heap > total` | +| Non-heap when `-Xmx` set | Squeezed to `total − Xmx` | Calculated independently | +| `-Xmx512M`, container=750M | ✅ passes | ❌ fails | -> **Note**: v4 correctly accounts for the full JVM memory footprint. When `-Xmx` is -> explicitly set, it is included in the total memory check — this is the right behaviour -> since the JVM will actually claim that heap in addition to native non-heap memory -> (thread stacks, metaspace, code cache). v3 silently squeezed non-heap into whatever -> remained after `-Xmx`, which could leave thread stacks and metaspace dangerously -> undersized at runtime. - -**Impact**: Apps that set `-Xmx` explicitly in `JAVA_OPTS` -**and** run in containers that cannot accommodate heap + non-heap will fail at startup with: +When `-Xmx` is not set, both v3 and v4 size heap and non-heap to fit within the container — no difference. When `-Xmx` is pinned, v4 requires the container to fit both heap and non-heap (thread stacks + metaspace + code cache). v3 squeezed non-heap into whatever remained after `-Xmx`, claiming less total memory — at the cost of potentially undersized thread stacks and metaspace at runtime. Apps that fit in smaller containers with v3 may fail at startup with v4: ``` required memory 1269289K is greater than 750M available for allocation ``` -The required memory is: `-Xmx` + thread stacks + metaspace + code cache. -With defaults (250 threads × ~1M = 250M stacks, ~100M metaspace, ~50M code cache): -a `-Xmx512M` app needs roughly **900–1000M** total. - -**Migration options** (choose one): - -1. **Remove `-Xmx` from `JAVA_OPTS`** — let the calculator size heap automatically. - This is the recommended approach; the calculator will allocate remaining memory - after reserving space for metaspace, code cache, and stack. +**Migration options**: -2. **Lower `stack_threads`** — the default 250 platform threads claim ~250M of native - memory. Reducing this is often the easiest way to fit within the container: +1. **Lower `stack_threads`** *(only if your app uses fewer than 250 threads)*: 250 threads × ~1M = ~250M native memory. Reducing this alone is often enough to fit within the container: ```yaml env: JBP_CONFIG_OPEN_JDK_JRE: '{ memory_calculator: { stack_threads: 50 } }' ``` - For apps using **virtual threads** (Java 21+, Spring Boot 3.2+), set this to the - carrier thread count (~CPU cores, typically 4–16). Virtual thread stacks are stored - on the heap as continuation objects, not as native stack memory, so only carrier - threads consume native stack memory. Note: with virtual threads you may want to - keep or increase `-Xmx` to accommodate heap-resident continuations. - -3. **Increase manifest memory** — raise `memory:` in `manifest.yml` to accommodate - the full JVM footprint (`-Xmx` + non-heap). With `-Xmx512M` and default thread - count, plan for ~900–1000M. + +2. **Remove `-Xmx` from `JAVA_OPTS`** — let the calculator size heap automatically. Note: removing `-Xmx` avoids the fixed-heap check but does not reduce total memory need. You will likely still need to increase `memory:` in `manifest.yml` so the calculator has enough room to allocate adequate heap. + +3. **Increase manifest memory** — raise `memory:` to fit `Xmx + non-heap`. Based on the error above (`1269289K ≈ 1240M`), set at least **1300M** for a `-Xmx512M` app with default settings. ### 10.3 Adoption Recommendations From 75bb0a5eca6e0ed698966ff1bca6132bff59ff6a Mon Sep 17 00:00:00 2001 From: Peter Paul Bakker Date: Wed, 6 May 2026 06:30:38 +0000 Subject: [PATCH 7/7] fix: LoadConfig reads correct JBP_CONFIG_*_JRE per vendor LoadConfig() hardcoded JBP_CONFIG_OPEN_JDK_JRE, silently ignoring memory calculator tuning (stack_threads, headroom, class_count) for all non-OpenJDK JREs (SapMachine, Zulu, IBM, Oracle, GraalVM). Pass jreName to NewMemoryCalculator so LoadConfig() resolves the correct env var via jreNameToDocumentedEnvVar. Restores the Ruby buildpack behaviour where each JRE read from its own config. Structural follow-up tracked in cloudfoundry/java-buildpack#1264. --- src/java/jres/graalvm.go | 4 +- src/java/jres/ibm.go | 4 +- src/java/jres/memory_calculator.go | 23 +++++--- .../jres/memory_calculator_issues_test.go | 52 ++++++++++++++++--- src/java/jres/openjdk.go | 4 +- src/java/jres/oracle.go | 4 +- src/java/jres/sapmachine.go | 4 +- src/java/jres/zulu.go | 4 +- 8 files changed, 75 insertions(+), 24 deletions(-) diff --git a/src/java/jres/graalvm.go b/src/java/jres/graalvm.go index 17ecdcfc5..86a9a6662 100644 --- a/src/java/jres/graalvm.go +++ b/src/java/jres/graalvm.go @@ -88,7 +88,7 @@ func (g *GraalVMJRE) Supply() error { } // Install Memory Calculator - g.memoryCalc = NewMemoryCalculator(g.ctx, g.jreDir, g.version, javaMajorVersion) + g.memoryCalc = NewMemoryCalculator(g.ctx, g.jreDir, g.version, javaMajorVersion, "graalvm") if err := g.memoryCalc.Supply(); err != nil { g.ctx.Log.Warning("Failed to install Memory Calculator: %s (continuing)", err.Error()) // Non-fatal - continue without memory calculator @@ -143,7 +143,7 @@ func (g *GraalVMJRE) Finalize() error { // Reconstruct Memory Calculator component if not already set if g.memoryCalc == nil { - g.memoryCalc = NewMemoryCalculator(g.ctx, g.jreDir, g.version, javaMajorVersion) + g.memoryCalc = NewMemoryCalculator(g.ctx, g.jreDir, g.version, javaMajorVersion, "graalvm") } // Finalize Memory Calculator diff --git a/src/java/jres/ibm.go b/src/java/jres/ibm.go index 5b61d4b4d..a6fa1662e 100644 --- a/src/java/jres/ibm.go +++ b/src/java/jres/ibm.go @@ -90,7 +90,7 @@ func (i *IBMJRE) Supply() error { } // Install Memory Calculator - i.memoryCalc = NewMemoryCalculator(i.ctx, i.jreDir, i.version, javaMajorVersion) + i.memoryCalc = NewMemoryCalculator(i.ctx, i.jreDir, i.version, javaMajorVersion, "ibm") if err := i.memoryCalc.Supply(); err != nil { i.ctx.Log.Warning("Failed to install Memory Calculator: %s (continuing)", err.Error()) // Non-fatal - continue without memory calculator @@ -146,7 +146,7 @@ func (i *IBMJRE) Finalize() error { // Reconstruct Memory Calculator component if not already set if i.memoryCalc == nil { - i.memoryCalc = NewMemoryCalculator(i.ctx, i.jreDir, i.version, javaMajorVersion) + i.memoryCalc = NewMemoryCalculator(i.ctx, i.jreDir, i.version, javaMajorVersion, "ibm") } // Finalize Memory Calculator diff --git a/src/java/jres/memory_calculator.go b/src/java/jres/memory_calculator.go index b08b03cfc..1d7199c52 100644 --- a/src/java/jres/memory_calculator.go +++ b/src/java/jres/memory_calculator.go @@ -19,6 +19,7 @@ type MemoryCalculator struct { jreDir string jreVersion string javaMajorVersion int + jreName string calculatorPath string version string classCount int @@ -31,12 +32,13 @@ type MemoryCalculator struct { } // NewMemoryCalculator creates a new memory calculator -func NewMemoryCalculator(ctx *common.Context, jreDir, jreVersion string, javaMajorVersion int) *MemoryCalculator { +func NewMemoryCalculator(ctx *common.Context, jreDir, jreVersion string, javaMajorVersion int, jreName string) *MemoryCalculator { return &MemoryCalculator{ ctx: ctx, jreDir: jreDir, jreVersion: jreVersion, javaMajorVersion: javaMajorVersion, + jreName: jreName, stackThreads: DefaultStackThreads, headroom: DefaultHeadroom, } @@ -384,15 +386,24 @@ type memoryCalculatorConfig struct { Headroom int `yaml:"headroom"` } -// LoadConfig loads memory calculator configuration from JBP_CONFIG_OPEN_JDK_JRE -// (standard CF format) and falls back to MEMORY_CALCULATOR_* env vars. +// LoadConfig loads memory calculator configuration from the JRE-specific env var +// (e.g. JBP_CONFIG_OPEN_JDK_JRE, JBP_CONFIG_SAP_MACHINE_JRE) and falls back +// to MEMORY_CALCULATOR_* env vars. // Must be called at the start of Supply(), before countClasses(). func (m *MemoryCalculator) LoadConfig() { if m.configLoaded { return } m.configLoaded = true - if config := os.Getenv("JBP_CONFIG_OPEN_JDK_JRE"); config != "" { + + // Resolve the env var name for this JRE (e.g. "sapmachine" → "JBP_CONFIG_SAP_MACHINE_JRE") + envVarName := jreNameToDocumentedEnvVar[m.jreName] + if envVarName == "" { + // fallback: auto-generate from jreName + envVarName = fmt.Sprintf("JBP_CONFIG_%s", strings.ToUpper(strings.ReplaceAll(m.jreName, "-", "_"))) + } + + if config := os.Getenv(envVarName); config != "" { yamlHandler := common.YamlHandler{} // Extract raw memory_calculator sub-section to validate its fields separately, @@ -404,14 +415,14 @@ func (m *MemoryCalculator) LoadConfig() { if err := yamlHandler.Unmarshal([]byte(config), &rawCfg); err == nil && rawCfg.MC != nil { if mcBytes, err := yamlHandler.Marshal(rawCfg.MC); err == nil { if err := yamlHandler.ValidateFields(mcBytes, &memoryCalculatorConfig{}); err != nil { - m.ctx.Log.Warning("Unknown fields in JBP_CONFIG_OPEN_JDK_JRE memory_calculator: %s", err.Error()) + m.ctx.Log.Warning("Unknown fields in %s memory_calculator: %s", envVarName, err.Error()) } } } cfg := openJDKJREConfig{} if err := yamlHandler.Unmarshal([]byte(config), &cfg); err != nil { - m.ctx.Log.Warning("Failed to parse JBP_CONFIG_OPEN_JDK_JRE: %s", err.Error()) + m.ctx.Log.Warning("Failed to parse %s: %s", envVarName, err.Error()) } else { mc := cfg.MemoryCalculator if mc.StackThreads > 0 { diff --git a/src/java/jres/memory_calculator_issues_test.go b/src/java/jres/memory_calculator_issues_test.go index b4621f961..bb21ca528 100644 --- a/src/java/jres/memory_calculator_issues_test.go +++ b/src/java/jres/memory_calculator_issues_test.go @@ -101,7 +101,7 @@ var _ = Describe("Memory Calculator Issues", func() { Expect(os.Setenv("MEMORY_CALCULATOR_STACK_THREADS", "50")).To(Succeed()) fakeBinary("4.2.0") - mc := jres.NewMemoryCalculator(ctx, jreDir, "17.0.9", 17) + mc := jres.NewMemoryCalculator(ctx, jreDir, "17.0.9", 17, "openjdk") Expect(mc.Finalize()).To(Succeed()) script := readGeneratedScript() @@ -118,7 +118,7 @@ var _ = Describe("Memory Calculator Issues", func() { Expect(os.Setenv("MEMORY_CALCULATOR_HEADROOM", "5")).To(Succeed()) fakeBinary("4.2.0") - mc := jres.NewMemoryCalculator(ctx, jreDir, "17.0.9", 17) + mc := jres.NewMemoryCalculator(ctx, jreDir, "17.0.9", 17, "openjdk") Expect(mc.Finalize()).To(Succeed()) script := readGeneratedScript() @@ -157,7 +157,7 @@ var _ = Describe("Memory Calculator Issues", func() { "{ memory_calculator: { stack_threads: 50 } }")).To(Succeed()) fakeBinary("4.2.0") - mc := jres.NewMemoryCalculator(ctx, jreDir, "17.0.9", 17) + mc := jres.NewMemoryCalculator(ctx, jreDir, "17.0.9", 17, "openjdk") Expect(mc.Finalize()).To(Succeed()) script := readGeneratedScript() @@ -179,7 +179,7 @@ var _ = Describe("Memory Calculator Issues", func() { "{ memory_calculator: { class_count: 3500 } }")).To(Succeed()) fakeBinary("4.2.0") - mc := jres.NewMemoryCalculator(ctx, jreDir, "17.0.9", 17) + mc := jres.NewMemoryCalculator(ctx, jreDir, "17.0.9", 17, "openjdk") Expect(mc.Finalize()).To(Succeed()) script := readGeneratedScript() @@ -193,6 +193,46 @@ var _ = Describe("Memory Calculator Issues", func() { }) }) + // ------------------------------------------------------------------------- + // JBP_CONFIG__JRE config is ignored for non-OpenJDK JREs + // + // LoadConfig() hardcodes JBP_CONFIG_OPEN_JDK_JRE, so memory calculator + // tuning via JBP_CONFIG_SAP_MACHINE_JRE, JBP_CONFIG_ZULU_JRE, etc. is + // silently ignored. + // + // Fix: pass jreName to NewMemoryCalculator and use jreNameToDocumentedEnvVar + // to read the correct env var in LoadConfig(). + // ------------------------------------------------------------------------- + Describe("Non-OpenJDK JRE config is silently ignored", func() { + It("JBP_CONFIG_SAP_MACHINE_JRE stack_threads is reflected in generated script", func() { + DeferCleanup(os.Unsetenv, "JBP_CONFIG_SAP_MACHINE_JRE") + Expect(os.Setenv("JBP_CONFIG_SAP_MACHINE_JRE", + "{ memory_calculator: { stack_threads: 50 } }")).To(Succeed()) + + fakeBinary("4.2.0") + mc := jres.NewMemoryCalculator(ctx, jreDir, "21.0.1", 21, "sapmachine") + Expect(mc.Finalize()).To(Succeed()) + + script := readGeneratedScript() + Expect(script).To(ContainSubstring("--thread-count=50"), + "expected --thread-count=50 from JBP_CONFIG_SAP_MACHINE_JRE but got default.\nScript:\n%s", script) + }) + + It("JBP_CONFIG_ZULU_JRE headroom is reflected in generated script", func() { + DeferCleanup(os.Unsetenv, "JBP_CONFIG_ZULU_JRE") + Expect(os.Setenv("JBP_CONFIG_ZULU_JRE", + "{ memory_calculator: { headroom: 10 } }")).To(Succeed()) + + fakeBinary("4.2.0") + mc := jres.NewMemoryCalculator(ctx, jreDir, "17.0.9", 17, "zulu") + Expect(mc.Finalize()).To(Succeed()) + + script := readGeneratedScript() + Expect(script).To(ContainSubstring("--head-room=10"), + "expected --head-room=10 from JBP_CONFIG_ZULU_JRE but got default.\nScript:\n%s", script) + }) + }) + // ------------------------------------------------------------------------- // JBP_CONFIG_OPEN_JDK_JRE field validation: // - Unknown top-level fields (e.g. jre:) must be silently ignored @@ -205,7 +245,7 @@ var _ = Describe("Memory Calculator Issues", func() { `{ jre: { version: "25.+" }, memory_calculator: { headroom: 5, stack_threads: 50 } }`)).To(Succeed()) fakeBinary("4.2.0") - mc := jres.NewMemoryCalculator(ctx, jreDir, "17.0.9", 17) + mc := jres.NewMemoryCalculator(ctx, jreDir, "17.0.9", 17, "openjdk") Expect(mc.Finalize()).To(Succeed()) script := readGeneratedScript() @@ -221,7 +261,7 @@ var _ = Describe("Memory Calculator Issues", func() { `{ memory_calculator: { stack_thread: 50 } }`)).To(Succeed()) // typo: missing 's' fakeBinary("4.2.0") - mc := jres.NewMemoryCalculator(ctx, jreDir, "17.0.9", 17) + mc := jres.NewMemoryCalculator(ctx, jreDir, "17.0.9", 17, "openjdk") Expect(mc.Finalize()).To(Succeed()) Expect(logBuffer.String()).To(ContainSubstring("WARNING"), diff --git a/src/java/jres/openjdk.go b/src/java/jres/openjdk.go index cf32acc91..5299d7261 100644 --- a/src/java/jres/openjdk.go +++ b/src/java/jres/openjdk.go @@ -113,7 +113,7 @@ func (o *OpenJDKJRE) Supply() error { } // Install Memory Calculator - o.memoryCalc = NewMemoryCalculator(o.ctx, o.jreDir, o.version, javaMajorVersion) + o.memoryCalc = NewMemoryCalculator(o.ctx, o.jreDir, o.version, javaMajorVersion, "openjdk") if err := o.memoryCalc.Supply(); err != nil { o.ctx.Log.Warning("Failed to install Memory Calculator: %s (continuing)", err.Error()) // Non-fatal - continue without memory calculator @@ -184,7 +184,7 @@ func (o *OpenJDKJRE) Finalize() error { // Reconstruct Memory Calculator component if not already set if o.memoryCalc == nil { - o.memoryCalc = NewMemoryCalculator(o.ctx, o.jreDir, o.version, javaMajorVersion) + o.memoryCalc = NewMemoryCalculator(o.ctx, o.jreDir, o.version, javaMajorVersion, "openjdk") } // Finalize Memory Calculator diff --git a/src/java/jres/oracle.go b/src/java/jres/oracle.go index 58c822f2b..f578f3b1b 100644 --- a/src/java/jres/oracle.go +++ b/src/java/jres/oracle.go @@ -89,7 +89,7 @@ func (o *OracleJRE) Supply() error { } // Install Memory Calculator - o.memoryCalc = NewMemoryCalculator(o.ctx, o.jreDir, o.version, javaMajorVersion) + o.memoryCalc = NewMemoryCalculator(o.ctx, o.jreDir, o.version, javaMajorVersion, "oracle") if err := o.memoryCalc.Supply(); err != nil { o.ctx.Log.Warning("Failed to install Memory Calculator: %s (continuing)", err.Error()) // Non-fatal - continue without memory calculator @@ -144,7 +144,7 @@ func (o *OracleJRE) Finalize() error { // Reconstruct Memory Calculator component if not already set if o.memoryCalc == nil { - o.memoryCalc = NewMemoryCalculator(o.ctx, o.jreDir, o.version, javaMajorVersion) + o.memoryCalc = NewMemoryCalculator(o.ctx, o.jreDir, o.version, javaMajorVersion, "oracle") } // Finalize Memory Calculator diff --git a/src/java/jres/sapmachine.go b/src/java/jres/sapmachine.go index 117ca3b19..1391d150e 100644 --- a/src/java/jres/sapmachine.go +++ b/src/java/jres/sapmachine.go @@ -88,7 +88,7 @@ func (s *SapMachineJRE) Supply() error { } // Install Memory Calculator - s.memoryCalc = NewMemoryCalculator(s.ctx, s.jreDir, s.version, javaMajorVersion) + s.memoryCalc = NewMemoryCalculator(s.ctx, s.jreDir, s.version, javaMajorVersion, "sapmachine") if err := s.memoryCalc.Supply(); err != nil { s.ctx.Log.Warning("Failed to install Memory Calculator: %s (continuing)", err.Error()) // Non-fatal - continue without memory calculator @@ -143,7 +143,7 @@ func (s *SapMachineJRE) Finalize() error { // Reconstruct Memory Calculator component if not already set if s.memoryCalc == nil { - s.memoryCalc = NewMemoryCalculator(s.ctx, s.jreDir, s.version, javaMajorVersion) + s.memoryCalc = NewMemoryCalculator(s.ctx, s.jreDir, s.version, javaMajorVersion, "sapmachine") } // Finalize Memory Calculator diff --git a/src/java/jres/zulu.go b/src/java/jres/zulu.go index 2ad1bc675..60ced44c6 100644 --- a/src/java/jres/zulu.go +++ b/src/java/jres/zulu.go @@ -88,7 +88,7 @@ func (z *ZuluJRE) Supply() error { } // Install Memory Calculator - z.memoryCalc = NewMemoryCalculator(z.ctx, z.jreDir, z.version, javaMajorVersion) + z.memoryCalc = NewMemoryCalculator(z.ctx, z.jreDir, z.version, javaMajorVersion, "zulu") if err := z.memoryCalc.Supply(); err != nil { z.ctx.Log.Warning("Failed to install Memory Calculator: %s (continuing)", err.Error()) // Non-fatal - continue without memory calculator @@ -143,7 +143,7 @@ func (z *ZuluJRE) Finalize() error { // Reconstruct Memory Calculator component if not already set if z.memoryCalc == nil { - z.memoryCalc = NewMemoryCalculator(z.ctx, z.jreDir, z.version, javaMajorVersion) + z.memoryCalc = NewMemoryCalculator(z.ctx, z.jreDir, z.version, javaMajorVersion, "zulu") } // Finalize Memory Calculator