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/6] 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/6] 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/6] 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/6] 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/6] 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/6] 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