gc: return free pages of surviving GC pools to the OS - #13
Conversation
ljmf00-wekaio
left a comment
ljmf00-wekaio
left a comment
There was a problem hiding this comment.
We need to obviously think more about this API if we want to upstream this, but for weka only, this looks good.
| __gshared bool g_stickyDisabled; | ||
| __gshared int g_status = Status.NOT_ARMED; | ||
| __gshared size_t g_poolCursor; // rotates which pool a pass starts scanning from | ||
| __gshared int g_injectFailMode; // test hook: 0=off, 1=next madvise(DONTNEED) fails, 2=next mlock2 fails |
There was a problem hiding this comment.
We are assuming that this is used in non multithreaded calls
There was a problem hiding this comment.
Correct, and I documented the invariant in 3193b78 rather than adding synchronisation.
All of this state is mutated only under the GC lock: the hooks are called from inside gc.d with the lock already held, and every extern(C) entry point that writes takes ConservativeGC.lockNR() the same way minimize() does. The exceptions are deliberate and racy-benign — weka_gc_dirty_free_bytes and weka_gc_scavenger_status are lock-free reads (a stale gauge or status code costs nothing: one feeds a policy heuristic, the other a stat), and weka_gc_scavenger_inject_fail is test-only.
If you would rather the counter were atomic I will do it, but I do not think a torn size_t is reachable given every writer holds the lock.
| else | ||
| private enum compiledOut = false; | ||
|
|
||
| private enum MIN_RUN_PAGES = 16; // 64KB -- below this, syscall/VMA churn isn't worth it |
There was a problem hiding this comment.
This is considering 4K pages, is this still ok for 64K page systems? I guess the GC is still hardcoded to 4K, so I would use the constant used in the GC instead of assuming 4K pages.
There was a problem hiding this comment.
Agreed, fixed in 3193b78. It is now MIN_RUN_BYTES = 64 * 1024 with MIN_RUN_PAGES = MIN_RUN_BYTES / PAGESIZE, using the GC's own PAGESIZE rather than assuming 4K, so the intent ("64KB is where the syscall and VMA churn stops being worth it") holds whatever the pool page size is.
There was a problem hiding this comment.
Coming back to this — your question was broader than my answer, and the real bug was not MIN_RUN_BYTES.
madvise/mlock2 operate in kernel pages. Aiming them at 4K-derived boundaries on a 64K host fails two ways: an unaligned start returns EINVAL, which scavengeRun misreads as "the heap is locked" and promotes the process to the munlock/mlock2 sequence; and the length is rounded up, past the end of the run, discarding live pages.
Fixed in the current tip: scavengerArm records sysconf(_SC_PAGESIZE), and scavengePool trims each run to its kernel-page-aligned interior, leaving the edges dirty — one page each, picked up when a neighbouring run grows over them. Arithmetically a no-op wherever the kernel page is the GC page. It fails closed to DISABLED_PAGE_SIZE_MISMATCH if sysconf fails, or reports a page below the GC's own, or one that is not a power of two.
There is a regression test that overrides the recorded granularity to 64K and asserts every reclaim is a whole number of kernel pages. It is mutation-verified: delete the trimming and it fails. Its blocks are deliberately 5 GC pages, because the first version used 64K blocks whose runs were accidentally already aligned, so it passed with the trimming removed.
Not yet covered: a live 64K host. Every machine I have is 4K, so that path rests on the unit test until I can get an aarch64 box with a 64K kernel.
3193b78 to
cdc3211
Compare
BaruchWeka
changed the title
f297774 to
2760cff
Compare
EyalIO
left a comment
EyalIO
left a comment
There was a problem hiding this comment.
I'm not really familiar enough with this code to give a proper review
|
@ljmf00-wekaio — you asked for two things: "think more about this API if we want to upstream this" here, and on the wekapp side "why don't we do this as part of minimize API (instead of a separate dedicated one) and only add thresholds in the reactor". Both are done, and this series is the result. Since it changes the shape you approved, worth a fresh look. The mechanism is now a phase of Also changed since your review:
On your thread-safety question: unchanged, and still lock-protected as described. The offer stands to make the counter atomic if you would rather not rely on the invariant. Upstream submission is gated on copyright assignment, not on anything technical. |
minimize() can only unmap a pool that is entirely free, so a single live object anywhere in a pool pins every page of it. A long-lived process whose heap grew once and then freed most of it therefore keeps paying for that peak in resident memory however much of each pool has since become free. Add a page-level scavenger for the conservative GC that releases the individual free pages of the pools minimize() had to keep, with madvise(MADV_DONTNEED), and run it as a final phase of minimize(). It is off unless enabled through the new gcopt scavenge option, and bounded by scavengeBudget and scavengeMinFree so an existing minimize() caller cannot inherit an unbounded syscall storm. Heaps that the process has locked into memory are supported. Where a plain madvise would be refused because the range is locked, the sequence becomes munlock/madvise(MADV_DONTNEED)/mlock2(MLOCK_ONFAULT), which drops the page from the resident set while keeping the mlockall(MCL_CURRENT|MCL_FUTURE) guarantee: the page is locked again at the moment it faults back in, so it is never swappable and the allocation site needs to know nothing. That case is detected from the kernel's EINVAL rather than configured. Linux only, and a no-op elsewhere: no other platform offers a way to drop a page out of RSS while keeping it locked.
The scavenge phase runs inside minimize() and cannot log -- it has no access to the host application's tracing -- so a live system had no way to tell whether it had acted, or which gate stopped it. Expose a ScavengeStats snapshot instead. A struct of named fields rather than a counter-by-index accessor: the caller reads one coherent snapshot instead of a sequence of independent reads, and adding a counter later cannot silently renumber the ones an existing caller already reads. Add a continuation hint alongside it. The phase stops on gcopt scavengeBudget, and nothing calls minimize() again on a quiet process, so record whether it left reclaimable pages behind rather than strand the remainder until the next caller happens along.
A host that wants to pace scavenging itself, rather than leave it to minimize(), needs to trigger a pass, read the dirty-free gauge and read back the phase stats. Expose them as extern(C) wrappers that resolve the live Gcx and take the GC lock, following the shape of ConservativeGC.minimize(). The lock-free readers are @nogc, so a host can call them directly from an @nogc context such as a statistics callback instead of caching their results. Also add the madvise/mlock2 failure injector the sticky-disable path is tested with, since it is only reachable from here. This is not the upstream-native shape -- that would be a GC interface method surfaced through core.memory, the way minimize() is, and would work under any GC rather than silently no-op under the others. Kept as a separate commit so it can be dropped or replaced without touching the mechanism.
Weka's processes lock their heap and depend on the scavenger to return it, so the gcopt option is on unless explicitly disabled rather than opt-in. Not for upstream: turning this on by default also turns MADV_NOHUGEPAGE on for every D program's GC heap, which is a throughput change nobody asked for.
2760cff to
dc711a2
Compare
3 participants
Returns whole free (
B_FREE) pool pages of the conservative GC to the OS, as a final phase ofminimize(), preserving anmlockall(MCL_CURRENT|MCL_FUTURE)guarantee where one exists.This branch was rewritten (force-push) into an upstream-shaped series: no
version (WEKA),no
weka-prefixed identifiers, gated by a newgcoptoption instead of a build version. Thepreviously-reviewed shape is preserved at
baruch/gc-scavenger-minimizeif you want to diff.See "What changed since the review" at the bottom.
Why
GC.minimize()can only unmap a pool that is 100 % free, so a heap that is mostly free butsparsely pinned never shrinks. Measured on a core dump from an internal long-running management
process (LDC 1.38, mlockall'd):
GC.minimize()could unmapNot one pool was fully free: 108 of 130 round to 0 % occupancy (64 MB pools held as little as
256 B in 2 objects), 120 of 130 are under 10 % occupied. Page-level scavenging recovers
≈7.35 GB there; whole-pool
munmaprecovers nothing. Sub-page slack — the part page scavengingcan never reach — is 7.4 MB, 0.1 %.
Mechanism
Plain
madvise(MADV_DONTNEED)per contiguous free run. On a heap the process has mlocked,madviseis refused, so the sequence becomesmunlock -> madvise(MADV_DONTNEED) -> mlock2(MLOCK_ONFAULT): the page leaves RSS but stayslocked, so when the GC re-carves it the first touch faults in a zero page that is locked at
fault time. No second fault, no swap exposure, and nothing at the allocation site needs to
know a page was ever scavenged — which is what keeps the hook surface this small. That case is
now detected from the kernel's
EINVALrather than configured, so a caller only enables thefeature; it never has to describe its own locking to the GC.
New module
core/internal/gc/scavenger.downs all of it: the per-poolscavengedMap, theglobal dirty-free-page counter, arm/disable state, the scavenge loop, a
vm.max_map_countpressure guard (partial munlock/mlock2 splits a pool's VMA, so near the ceiling the split itself
can
ENOMEMmid-sequence), and sticky-disable-on-syscall-failure with a best-effort re-lock.impl/conservative/gc.dgets one field onPooland nine one-line hook calls — everyB_FREEtransition and pool lifecycle point (
Pool.initialize,Pool.Dtor, both sweep branches,LargeObjectPool.freePages/allocPages,SmallObjectPool.allocPage,extendNoSync,reallocNoSync's expand-in-place carve) — plus theminimize()call site.Configuration
Off unless enabled. Four
gcoptoptions:scavenge0upstream,1in this forkminimize()scavengeMinFreescavengeBudgetminimize()may release; call again for morescavengeNoHugePages1MADV_NOHUGEPAGEtracked pools — page-granular release needs itThe default flip lives in its own clearly-marked fork-only commit, because defaulting
MADV_NOHUGEPAGEon for every D program's GC heap is a throughput change nobody opted into.The series
druntime: return free pages of surviving GC pools to the OS— module, hooks, gcopt options,the
minimize()phase, arm-on-first-use, unittest, changelog entrydruntime: phase stats and a continuation hint for the GC scavenge phase— the phase runs insideminimize()and cannot log, so a live system reads aScavengeStatssnapshot insteaddruntime: extern(C) entry points for application-driven GC scavenging— for a host thatwants to pace scavenging itself, plus the failure injector
Weka druntime modification: enable GC page scavenging by default— fork onlyThe
extern(C)surface, renamed offweka_gc_*:The lock-free readers are
@nogcso a host can call them from an@nogccontext — a statisticscallback, say — rather than caching their results.
Deliberately not plain
gc_*: that is druntime's public C ABI for the GC (gc_malloc,gc_minimize, …), routed through the registry indirection, and agc_scavengeadded frominside the conservative implementation would join that namespace while silently no-op'ing under
every other GC. Commit 3 says so in its own message — the upstream-native form is a
GCinterface method surfaced through
core.memory, the wayminimize()is, and commit 3 is keptseparate so it can be replaced without touching the mechanism.
Validation
ldc-build-runtime --testrunners: the scavengermodule passes, as does every GC module with unittests including
core.internal.gc.impl.conservative.gc, which exercises the added hooks.GC.minimize()growsScavengeStats.scavengedBytesby exactly the dirty-free bytes it started with, sothe whole drop is attributed to the scavenge phase and not to a pool being unmapped whole.
Mutating the phase to return early makes dirty-free not fall at all and fails the assertion.
with
VmLckheld at ~1.3 GB, i.e. the lock guarantee really is kept); live multi-node clusterwith a debug-assert build — forced and automatic passes both reclaim, RSS drop equals reported
scavenged bytes, map count stays ≈1 VMA per pool in steady state, zero process restarts.
What changed since the review
version (WEKA)gone from all 13 scavenger sites; the mechanism isversion (linux)and thefeature is
gcopt. This answers review ask 3 ("is theversion (WEKA)gate the right shape,or should it be a runtime switch with an eye to upstreaming") — it is now a runtime switch.
wekaScavengerX→scavengerX,Status→ScavengeStatus,the
Counterenum → aScavengeStatsstruct,weka_gc_*→gc_scavenger_*. Header isCopyright: D Language Foundationper the surrounding modules.minimize()— the pools that survive its whole-pool phase areexactly the mostly-free-but-pinned ones this exists for — and arms itself on first use.
static assert(false)on the mlock2 syscall numberbroke the build on every Linux arch except x86_64/AArch64.
gc.dimports the moduleunconditionally, so a druntime build for ARM32, RISC-V, PPC64 or s390x died outright. Now
SYS_mlock2 = -1with ahaveMlock2guard: only the locked-heap sequence is unavailablethere, and an unlocked heap scavenges normally.
Gcx.instanceisversion (Posix)-only, so theextern(C)block broke the Windows build; nowversion (Posix):-scoped.vm.max_map_countis read once instead of on every pass, and the map-pressure check is skippedentirely unless the heap is mlocked —
MADV_DONTNEEDchanges no VMA flags and so cannot splita VMA; only
munlock/mlock2can.Upstream
druntimeupstream isdlang/dmd(druntime/), not LDC — LDC's copy is a sync. All hunks ofcommits 1–3 apply to current dmd master with offsets only; the sole exception is the
extern(C)block's end-of-file anchor, which moved. A
changelog/druntime.gc-page-scavenging.ddentry isincluded for that PR. We are not waiting for upstream before merging here; incompatibilities
get fixed when LDC next syncs druntime.
Review asks
B_FREEtransition or pool-lifecycle point missed? Thatis the one class of bug that would under-count rather than misbehave visibly.
extern(C)entry points (modeled onConservativeGC.minimize).scavengedefaulting on in this fork (commit 4) the right call, given it also turnsMADV_NOHUGEPAGEon for the whole GC heap?v1.38.0-weka16tag, so consumers can pin a tag instead of this SHA.