jamesfredley opened a new pull request, #16168:
URL: https://github.com/apache/grails-core/pull/16168
> [!NOTE]
> Stacked on #16158 - this branch is based on it, so review that one first.
The diff here is only the heap change.
## What
Replace the hardcoded per-fork test heap with an explicit budget derived
from the machine.
`gradle/test-config.gradle` has always said:
```groovy
maxHeapSize = isCiBuild ? '768m' : '1024m'
```
Those literals ignore the machine. Every forked test JVM claimed a gigabyte
locally no matter how much RAM the developer actually had, and nothing tied
that number to how many forks could be alive at once.
## The bound
Forked test JVMs are children of the daemon, so `org.gradle.jvmargs` does
not limit them. With `org.gradle.parallel=true` several `Test` tasks run
concurrently, so the live fork count is bounded by Gradle's **build-wide worker
pool** (`maxWorkerCount`), not by any one task's `maxParallelForks`. A build
with four tasks capped at two forks apiece still runs up to `maxWorkerCount`
forks at once, so budgeting off the per-task cap would hand every fork a heap
the machine cannot honour.
This is the same bound - and the same reasoning - that
`ActiveProcessorCountArgumentProvider` already uses for CPU count a few lines
below in `build.gradle`. This PR applies it to memory.
The budget: physical RAM, minus the daemon's own heap, halved to leave room
for the OS, the compiler workers (`-Xmx2G` each via `CompilePlugin`) and the
Docker containers the mongodb/redis/geb suites start per fork, divided by the
worker count, clamped to `[512, 1024]`.
The daemon heap is read from `Runtime.runtime.maxMemory()` rather than
parsed out of `org.gradle.jvmargs` - the build script runs *in* the daemon, so
this sidesteps multiple `-Xmx` options (the JVM honours the last) and
unit-parsing entirely.
## CI is unchanged, by construction
On CI the value is still exactly `768`. This is load-bearing: the PR makes
the policy explicit and fixes **local** overcommit only. Changing CI numbers
belongs in a later, measured PR - mixing them here would make any CI flake
impossible to attribute ("was it the heap or the CPU cap?").
Both `-PtestForkHeapMb` and `isCiBuild` settle the value *before* the probe
runs, so the CI path performs no fallible work at all.
## Measured
Real resolved `Test` task properties, captured with a throwaway init script
(since deleted), not hand arithmetic:
| Scenario | workers | forks | `maxHeapSize` |
|---|---|---|---|
| local (64 GB, `-Xmx5G` daemon) | 20 | 10 | `1024m` (ceiling) |
| `CI=true` | 20 | 4 | **`768m`** |
| `CI=true -PmaxTestParallel=4` | 20 | 4 | **`768m`** |
| `-PtestForkHeapMb=900` | 20 | 10 | `900m` |
| `CI=true -PtestForkHeapMb=900` | 20 | 4 | `900m` |
| local `--max-workers=64` | 64 | 10 | `512m` (floor) |
Verified on both `grails-core:test` and `grails-test-suite-uber:test`.
## grails-test-suite-uber
That module carried its **own copy** of the same `768m`/`1024m` literal and
never applied `gradle/test-config.gradle`, so the heaviest suite in the build
would have been the one module exempt from the budget. It now reads the same
property. `grails-test-suite-persistence` keeps its deliberate `2048m` - that
is a specialized limit, not a copy-paste default.
## Known limit (deliberately not papered over)
When the computed share falls below the 512m floor, the floor wins and the
forks collectively still exceed the budget. Heap alone cannot fix that: a fork
below ~512m cannot run these suites. The knob that has to come down on such a
host is the **fork count** (`-PmaxTestParallel`, or `--max-workers` which
bounds them build-wide). This budget lowers memory pressure; it does not by
itself prove a constrained machine fits. The code says so where it clamps.
## Why draft
The constants are judgement calls, not measurements, and I would rather
agree the policy than defend the numbers:
- Is `512` a safe floor for the heaviest suites, or should the fork count
drop instead once the share falls that low?
- Should the reserve be "half the remainder", or should the `-Xmx2G`
compiler workers be subtracted explicitly?
## Scope
- Root build only. `grails-gradle` and `grails-forge` are separate Gradle
builds with their own daemon settings, their own `configuredTestParallel` and
their own heap literals - deliberately a follow-up, not silent collateral.
- No test is added, removed, skipped or weakened. Heap sizing does not
change test selection.
- Reading physical RAM degrades safely: a failed probe keeps the previous
`1024`, and a *successful* probe on a machine whose RAM sits inside the daemon
heap drops to the floor rather than pretending the probe failed.
## Verification
- `./gradlew help` - EXIT 0
- `CI=true ./gradlew help -PmaxTestParallel=4` - EXIT 0
- `./gradlew help -PtestForkHeapMb=900` - EXIT 0
- `./gradlew validateActions` - EXIT 0
## Related
Follow-up to #16158 (CPU oversubscription) and #16167 (daemon heap vs runner
RAM). Three separate knobs, deliberately three separate PRs: CPU, daemon heap,
fork heap.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
