From 512587f04688b044cf960728bab64b89cfd647b9 Mon Sep 17 00:00:00 2001 From: Martin Vogel Date: Sat, 25 Jul 2026 12:53:07 +0200 Subject: [PATCH] wip: periodic windows heap trim Signed-off-by: Martin Vogel --- src/foundation/mem.c | 42 ++++++++++++++++++++++++++++++++++++++++++ src/foundation/mem.h | 7 +++++++ src/mcp/mcp.c | 11 +++++++++++ 3 files changed, 60 insertions(+) diff --git a/src/foundation/mem.c b/src/foundation/mem.c index 617a2fe21..336f040d9 100644 --- a/src/foundation/mem.c +++ b/src/foundation/mem.c @@ -28,6 +28,7 @@ #endif #include #include +#include #elif defined(__APPLE__) #include #else @@ -333,6 +334,47 @@ size_t cbm_mem_worker_budget(int num_workers) { void cbm_mem_collect(void) { mi_collect(true); +#ifdef _WIN32 + /* mi_collect only returns pages the mimalloc heaps own. On Windows the + * static-CRT override does not capture every allocation — a heap walk of a + * process holding tens of megabytes reports zero live blocks — so those + * allocations are freed to the CRT heap instead, which keeps the pages + * committed rather than returning them. The purge_delay=0 configuration + * above therefore never applies to them, which is precisely why repeated + * request-scoped store open/close ratcheted committed memory on Windows + * only while POSIX stayed flat (#581). _heapmin asks the CRT to hand its + * unused pages back, which is the missing half of "return unused pages to + * the OS" on this platform. */ + (void)_heapmin(); +#endif +} + +void cbm_mem_collect_periodic(void) { +#ifdef _WIN32 + /* Platform parity, not whack-a-mole. On POSIX every free can return its + * pages immediately (purge_delay=0 above), so a long-lived process stays + * flat no matter which code path allocated. Windows has no equivalent for + * CRT-heap memory, so trimming only at one call site leaves every other + * path ratcheting — the residual growth #581 still showed after the + * request-scoped store churn was fixed. A bounded periodic trim gives the + * whole process the same "freed means returned" property. + * + * Rate-limited because _heapmin walks the heap: at most once per interval, + * driven from the server's existing idle/maintenance tick, so a busy + * request loop never pays it more than once per second. */ + enum { MEM_TRIM_MIN_INTERVAL_MS = 1000 }; + static atomic_llong last_trim_ms = ATOMIC_VAR_INIT(0); + long long now = (long long)cbm_now_ms(); + long long previous = atomic_load_explicit(&last_trim_ms, memory_order_relaxed); + if (previous != 0 && now - previous < MEM_TRIM_MIN_INTERVAL_MS) { + return; + } + if (!atomic_compare_exchange_strong_explicit(&last_trim_ms, &previous, now, + memory_order_acq_rel, memory_order_relaxed)) { + return; /* another thread is trimming this interval */ + } + (void)_heapmin(); +#endif } /* ── Memory map (see mem.h for how to read the triple) ─────────────── */ diff --git a/src/foundation/mem.h b/src/foundation/mem.h index 43f52237f..eeef2cdb0 100644 --- a/src/foundation/mem.h +++ b/src/foundation/mem.h @@ -78,6 +78,13 @@ size_t cbm_mem_worker_budget(int num_workers); /* Return unused pages to the OS. Call between files to bound per-file peak. */ void cbm_mem_collect(void); +/* Bounded, rate-limited page return for a long-lived process's maintenance + * tick. No-op on POSIX, where every free already returns its pages + * (purge_delay=0); on Windows it trims the CRT heap, which otherwise keeps + * freed pages committed no matter which code path freed them (#581). Safe to + * call from a hot loop: at most one trim per second, process-wide. */ +void cbm_mem_collect_periodic(void); + /* ── Memory map: where does the process's memory actually live? ────── * * A growth diagnostic that is honest about what it cannot see. Walking the diff --git a/src/mcp/mcp.c b/src/mcp/mcp.c index 36afeb6f8..7792dc81d 100644 --- a/src/mcp/mcp.c +++ b/src/mcp/mcp.c @@ -10419,6 +10419,14 @@ static void release_request_store(cbm_mcp_server_t *srv) { srv->store = NULL; free(srv->current_project); srv->current_project = NULL; + /* The close above frees a whole connection's worth of page cache and + * schema. Handing those pages back to the OS is what keeps a long-lived + * daemon flat across thousands of request-scoped stores (#581): without + * it, committed memory ratcheted ~2 MB per request on Windows, where freed + * CRT pages stay committed, while POSIX stayed flat because the allocator + * is configured to purge immediately. Placed here rather than inside + * cbm_store_close so only this request-scoped churn pays for it. */ + cbm_mem_collect(); } char *cbm_mcp_handle_tool(cbm_mcp_server_t *srv, const char *tool_name, const char *args_json) { @@ -11130,6 +11138,7 @@ static int poll_for_input_unix(cbm_mcp_server_t *srv, int fd, FILE *in) { } if (pr == 0) { cbm_mcp_server_evict_idle(srv, STORE_IDLE_TIMEOUT_S); + cbm_mem_collect_periodic(); return 0; } return SKIP_ONE; @@ -11158,6 +11167,7 @@ static int poll_for_input_unix(cbm_mcp_server_t *srv, int fd, FILE *in) { } if (pr == 0) { cbm_mcp_server_evict_idle(srv, STORE_IDLE_TIMEOUT_S); + cbm_mem_collect_periodic(); return 0; } return SKIP_ONE; @@ -11201,6 +11211,7 @@ int cbm_mcp_server_run(cbm_mcp_server_t *srv, FILE *in, FILE *out) { } if (wr == WAIT_TIMEOUT) { cbm_mcp_server_evict_idle(srv, STORE_IDLE_TIMEOUT_S); + cbm_mem_collect_periodic(); continue; } #else