diff --git a/src/adapter-merge.h b/src/adapter-merge.h index 3108b54..3816927 100644 --- a/src/adapter-merge.h +++ b/src/adapter-merge.h @@ -467,6 +467,18 @@ static bool adapter_merge_on_backend(WeightCtx * pc->src = merged_buf; pc->nbytes = merged_bytes; } + // The PendingCopy keeps the file range it was pushed with (file_src/file_nbytes/ + // file_base/file_len, untouched by the swap above), so wctx_alloc releases the + // base weight's staged pages exactly as it does for an unmerged tensor -- an + // adapter load gets the same residency reduction as a plain one, and the release + // shows up in its "unmapped N MB" line. Nothing is released here, mid-merge: the + // pages are only dead once every pending entry has been copied. + // + // A second merge targeting the same base tensor is last-wins and safe as-is: it + // re-reads the pristine base from the mapping (the merge never writes back to + // base_ptr), recomputes, and replaces pc->src. Within one pass that cannot happen + // anyway -- both merge paths visit each base tensor once -- and merges always run + // before wctx_alloc, so the released pages are never read here. wctx->staging.push_back(std::move(staging_buf)); ggml_backend_buffer_free(buf); @@ -962,6 +974,19 @@ static bool adapter_merge(WeightCtx * wctx, const char * adapter_path, float scale, ggml_backend_t backend) { + // Merges read the base weights straight out of gf.mapping, which wctx_alloc + // punches holes in as it copies -- so the ordering (merge first, alloc second) + // is a correctness rule, not a preference. Enforce it -- but gracefully: the one + // caller already turns a false return into a clean failed load, which beats + // exiting the process (the default build) or unwinding mid-request (the app). + // Once the buffer exists some source pages may be released and the upload could + // fault, so refuse to run rather than guess. + if (wctx->buffer != nullptr) { + fprintf(stderr, + "[Adapter] FATAL: adapter merge after wctx_alloc; the base weight pages may already be " + "released\n"); + return false; + } std::string sf_path; std::string cfg_dir; diff --git a/src/dit.h b/src/dit.h index 54f7f8f..b57946f 100644 --- a/src/dit.h +++ b/src/dit.h @@ -422,7 +422,10 @@ static bool dit_ggml_load(DiTGGML * m, if (adapter_path) { Timer adapter_timer; if (!adapter_merge(&m->wctx, gf, adapter_path, adapter_scale, m->backend)) { - fprintf(stderr, "[Adapter] FATAL: no tensors merged (model mismatch)\n"); + // adapter_merge already printed the specific reason (bad path, layout, + // no matching tensors, or a merge-after-alloc ordering bug) -- a generic + // "model mismatch" here would send an operator re-exporting a good LoRA. + fprintf(stderr, "[Adapter] FATAL: merge failed; see the [Adapter] messages above\n"); gf_close(&gf); return false; } diff --git a/src/gguf-weights.h b/src/gguf-weights.h index 6adc2b0..ec99611 100644 --- a/src/gguf-weights.h +++ b/src/gguf-weights.h @@ -16,8 +16,10 @@ #include "gguf.h" #include "weight-ctx.h" +#include #include #include +#include #include #ifdef _WIN32 @@ -30,11 +32,11 @@ #endif struct GGUFModel { - struct gguf_context * gguf; // parsed header (KV + tensor metadata) - struct ggml_context * meta; // tensor descriptors (no data) - uint8_t * mapping; // mmapped file - size_t file_size; - size_t data_offset; // gguf_get_data_offset(gguf) + struct gguf_context * gguf = nullptr; // parsed header (KV + tensor metadata) + struct ggml_context * meta = nullptr; // tensor descriptors (no data) + uint8_t * mapping = nullptr; // mmapped file + size_t file_size = 0; + size_t data_offset = 0; // gguf_get_data_offset(gguf) #ifdef _WIN32 HANDLE fh; HANDLE mh; @@ -62,7 +64,16 @@ static void gf_close(GGUFModel * gf) { } #else if (gf->mapping) { - munmap(gf->mapping, gf->file_size); + // After a 2.7 load this range carries interior holes where wctx_alloc already + // released staged pages. Unmapping over holes works on macOS and Linux + // (verified), but hole tolerance is implementation behaviour rather than a + // POSIX guarantee -- so a failure here means the whole mapping stays resident + // until process exit. Plain fprintf, not warn-once: this runs once per model, + // and every failing close is a distinct mapping worth its own line. + if (munmap(gf->mapping, gf->file_size) != 0) { + fprintf(stderr, "[GGUF] WARNING: munmap of the model mapping failed (%s); its pages stay resident\n", + strerror(errno)); + } } if (gf->fd >= 0) { close(gf->fd); @@ -153,6 +164,20 @@ static bool gf_load(GGUFModel * gf, const char * path) { return true; } +// Queue a copy straight from the file mapping, recording where the bytes came from so +// wctx_alloc can release those staged pages once copied (Apple: munmap, Linux: +// madvise). The recorded range survives an adapter merge repointing src at a heap +// staging buffer, so the original pages are still released. Copies from heap staging +// or the stack push PendingCopy directly -- they have nothing to release. +static void wctx_push_file_copy(WeightCtx * wctx, + struct ggml_tensor * tensor, + const void * file_src, + size_t nbytes, + size_t offset, + const GGUFModel & gf) { + wctx->pending.push_back(wctx_make_file_copy(tensor, file_src, nbytes, offset, gf.mapping, gf.file_size)); +} + // Load a tensor from GGUF into the weight context. // Returns ggml_tensor (not yet backed by memory; call wctx_alloc after all loads). // Tensor shapes are already in ggml order (ne[0]=innermost). @@ -196,7 +221,7 @@ static struct ggml_tensor * gf_load_tensor(WeightCtx * wctx, const void * data = gf.mapping + gf.data_offset + offset; size_t nbytes = ggml_nbytes(src); - wctx->pending.push_back({ tensor, data, nbytes, 0 }); + wctx_push_file_copy(wctx, tensor, data, nbytes, 0, gf); return tensor; } @@ -266,6 +291,17 @@ static struct ggml_tensor * gf_load_tensor_f32(WeightCtx * wctx, const GGUFModel // Get raw pointer to tensor data in the mmapped file. // Useful for CPU-side operations (e.g. bf16 embed lookup for lyrics). // Returns NULL if not found. +// +// Lifetime: the pointer is valid only while this GGUFModel is open AND before any +// wctx_alloc has run on a WeightCtx loading from it -- on Apple wctx_alloc unmaps +// each copied tensor's pages as it goes (weight-ctx.h), so a late read of a copied +// tensor now faults instead of harmlessly refaulting the page. Current users are safe +// for different reasons, and both matter: model-store metadata and store_silence open +// their own short-lived mapping that no WeightCtx touches, and vae.h reads its mapping +// across phases but never calls wctx_alloc at all -- nothing releases its pages. Note +// the second half: a module may read before OR after its own buffer allocation; the +// rule is only about WeightCtx's release, so a module converted TO the WeightCtx +// idiom must move every post-alloc mapping read into the push/copy flow. static const void * gf_get_data(const GGUFModel & gf, const char * name) { int64_t idx = gguf_find_tensor(gf.gguf, name); if (idx < 0) { @@ -312,10 +348,9 @@ static struct ggml_tensor * gf_load_qkv_fused(WeightCtx * wctx, size_t off = gguf_get_tensor_offset(gf.gguf, idx); return gf.mapping + gf.data_offset + off; }; - - wctx->pending.push_back({ fused, get_data(q_name), q_bytes, 0 }); - wctx->pending.push_back({ fused, get_data(k_name), k_bytes, q_bytes }); - wctx->pending.push_back({ fused, get_data(v_name), v_bytes, q_bytes + k_bytes }); + wctx_push_file_copy(wctx, fused, get_data(q_name), q_bytes, 0, gf); + wctx_push_file_copy(wctx, fused, get_data(k_name), k_bytes, q_bytes, gf); + wctx_push_file_copy(wctx, fused, get_data(v_name), v_bytes, q_bytes + k_bytes, gf); return fused; } @@ -347,9 +382,8 @@ static struct ggml_tensor * gf_load_pair_fused(WeightCtx * wctx, size_t off = gguf_get_tensor_offset(gf.gguf, idx); return gf.mapping + gf.data_offset + off; }; - - wctx->pending.push_back({ fused, get_data(a_name), a_bytes, 0 }); - wctx->pending.push_back({ fused, get_data(b_name), b_bytes, a_bytes }); + wctx_push_file_copy(wctx, fused, get_data(a_name), a_bytes, 0, gf); + wctx_push_file_copy(wctx, fused, get_data(b_name), b_bytes, a_bytes, gf); return fused; } diff --git a/src/qwen3-lm.h b/src/qwen3-lm.h index feec651..af0811c 100644 --- a/src/qwen3-lm.h +++ b/src/qwen3-lm.h @@ -316,7 +316,15 @@ static bool qw3lm_load(Qwen3LM * m, const char * gguf_path, int max_seq_len, int qwen3_load_layer(&m->wctx, gf, &m->layers[i], prefix, i); } - wctx_alloc(&m->wctx, m->backend); + // The only wctx_alloc caller that may not discard the bool: a failed backend + // allocation leaves buffer == NULL, and a module that reports success anyway + // dereferences it on the first prefill. (Every other loader checks.) No + // self-teardown on this failure: store_require_lm's LoadGuard owns the + // module, same as the other five loaders, and gf_close below is still + // reached on the way out. + if (!wctx_alloc(&m->wctx, m->backend)) { + return false; + } gf_close(&gf); // KV cache diff --git a/src/safetensors.h b/src/safetensors.h index e3d3398..049d9d9 100644 --- a/src/safetensors.h +++ b/src/safetensors.h @@ -8,6 +8,7 @@ // Only handles the flat safetensors JSON structure. // Not a general purpose JSON parser. +#include #include #include #include @@ -305,7 +306,16 @@ static void st_close(STFile * st) { } #else if (st->mapping) { - munmap(st->mapping, st->file_size); + // Adapter payloads come from user-supplied files, so this is the likeliest + // unmap to actually fail; a silent failure would leave the mapping resident + // until process exit with nothing on stderr. Runs once per file, so a plain + // line per failure is right. + if (munmap(st->mapping, st->file_size) != 0) { + fprintf(stderr, + "[Safetensors] WARNING: munmap of the adapter mapping failed (%s); its pages stay " + "resident\n", + strerror(errno)); + } } if (st->fd >= 0) { close(st->fd); diff --git a/src/weight-ctx.h b/src/weight-ctx.h index e43f573..43b089e 100644 --- a/src/weight-ctx.h +++ b/src/weight-ctx.h @@ -9,15 +9,45 @@ // wctx_init(&wctx, n_tensors); // ggml_tensor * w = _load_tensor(&wctx, source, "name"); // wctx_alloc(&wctx, backend); +// +// Loaders that copy straight out of a file mapping record where the bytes came +// from (wctx_push_file_copy in gguf-weights.h); wctx_alloc releases those staged +// pages as it copies (Apple: munmap, Linux: madvise), which is what keeps load-time +// page residency near one tensor instead of the whole file. #include "ggml-backend.h" #include "ggml.h" #include +#include #include +#include +#include #include #include +// One decision point for "this platform has a staged-page release mechanism" +// (munmap on Apple, madvise on Linux): the include block, the release body and the +// completion report below all key off it, so adding a platform is one edit. +#if defined(__APPLE__) || defined(__linux__) +# define ACE_HAS_PAGE_RELEASE 1 +#endif + +#ifdef ACE_HAS_PAGE_RELEASE +# ifdef __APPLE__ +# include // munmap +# else +# include // madvise, MADV_DONTNEED +# endif +# include // sysconf, _SC_PAGESIZE +#endif + +#include // errno, for the release-failure warning + +#ifdef ACE_HAS_PAGE_RELEASE +# include +#endif + struct WeightCtx { struct ggml_context * ctx; ggml_backend_buffer_t buffer; @@ -27,6 +57,27 @@ struct WeightCtx { const void * src; size_t nbytes; size_t offset; // byte offset into dst tensor (0 for regular loads) + + // Where the bytes came from, recorded at push time by the loaders that copy + // straight out of a file mapping (wctx_push_file_copy in gguf-weights.h): the + // original mapping address and extent to release once this copy is done. Null + // for copies from heap staging or the stack, which have nothing to release. + // Kept separate from src/nbytes so an adapter merge can repoint src at its + // merged heap staging and the original mapping pages still get released by + // wctx_alloc -- no second mapping parameter to mis-pair at the call site. + // + // Invariant: two PendingCopys must never name overlapping file ranges. The + // first release unmaps the pages, and the second copy's read then faults on + // Apple (on Linux madvise would silently refault, hiding the same bug). Tied + // weights -- one GGUF byte range serving two tensors -- are the natural way + // to violate this; push one copy and reuse the tensor instead. Loaders that + // convert out of the mapping (norms, biases, the pre-permutes) push plain + // copies with these fields null; their (small) source pages release at + // gf_close instead, which is part of the gap the completion line reports. + const void * file_src = nullptr; + size_t file_nbytes = 0; + const void * file_base = nullptr; + size_t file_len = 0; }; std::vector pending; @@ -37,6 +88,20 @@ struct WeightCtx { std::vector> staging; }; +// The one place a file-backed PendingCopy is built, named beside the struct it +// names: every field is a pointer or a size, so a transposed 8-element positional +// literal would compile and silently break release. Only wctx_push_file_copy +// (gguf-weights.h) constructs file copies; everything else -- heap staging, stack +// scalars -- pushes the 4-field form and gets no release by definition. +inline WeightCtx::PendingCopy wctx_make_file_copy(struct ggml_tensor * tensor, + const void * file_src, + size_t nbytes, + size_t offset, + const void * file_base, + size_t file_len) { + return { tensor, file_src, nbytes, offset, file_src, nbytes, file_base, file_len }; +} + static void wctx_init(WeightCtx * wctx, int n_tensors) { size_t ctx_size = (size_t) n_tensors * ggml_tensor_overhead() + 1024; struct ggml_init_params params = { @@ -50,22 +115,224 @@ static void wctx_init(WeightCtx * wctx, int n_tensors) { wctx->pending.reserve(n_tensors); } +// The ACE_NO_PAGE_UNMAP escape hatch for a destructive op in the load path: set it to +// any non-empty value other than "0" to keep the whole mapping resident until gf_close +// (the pre-2.7 behaviour), for A/B footprint measurement or to rule this out if a +// device ever misbehaves. "=0" or unset means release runs. This is a diagnostic +// switch, deliberately not part of the AceBackendConfig API: an application has no +// business disabling memory release, it exists for measurement and emergencies. +// Deliberately NOT cached: the A/B use case flips it between loads in one process, so +// a latched value would make the second leg measure the first. getenv is a linear +// scan of the environment -- nothing against a multi-GB weight copy. +static bool wctx_page_unmap_disabled(void) { + const char * v = std::getenv("ACE_NO_PAGE_UNMAP"); + return v != nullptr && v[0] != '\0' && strcmp(v, "0") != 0; +} + +#ifdef ACE_HAS_PAGE_RELEASE +// The per-platform release primitive over a page-aligned interior range. Same +// contract on both: drop the pages, return false + set errno on failure. +# if defined(__APPLE__) +// munmap is the only call that works on macOS: the madvise family (POSIX_MADV_DONTNEED, +// MADV_DONTNEED, MADV_FREE) is a no-op there -- measured with mincore, they free +// nothing. munmap is destructive (an unexpected re-read faults instead of refaulting), +// which is why the containment and interior-only rules in the caller matter. +static bool wctx_release_pages(void * start, size_t len) { + return munmap(start, len) == 0; +} + +inline const char * wctx_release_op = "munmap"; // inline: odr-used by the inline warn-once below +# else +// Linux: madvise(MADV_DONTNEED) does work on file mappings there and keeps the +// mapping intact -- same drop-and-refault semantics for these read-only staged pages. +static bool wctx_release_pages(void * start, size_t len) { + return madvise(start, len, MADV_DONTNEED) == 0; +} + +inline const char * wctx_release_op = "madvise"; // inline: odr-used by the inline warn-once below +# endif + +// Release the staged GGUF mmap pages a just-copied tensor occupied: the whole pages +// fully inside its byte range. The mmap is staging, not residency (gguf-weights.h +// copies each tensor into the backend buffer, then gf_close releases the rest), so +// once ggml_backend_tensor_set has consumed a tensor its file pages are dead weight. +// Releasing them per tensor keeps peak clean-page residency across a large load near +// one tensor instead of the whole file; gf_close later releases the whole original +// range, which tolerates these interior holes. +// +// Copy-before-release: wctx_alloc calls ggml_backend_tensor_set -- the synchronous +// variant, never ggml_backend_tensor_set_async. The buffer-level iface it dispatches +// to (ggml_backend_buffer_i::set_tensor) has no deferred form; the async operations +// that exist live on the backend-level ggml_backend_i and are separate API calls +// this code does not use. Verified per vendored backend that the synchronous call +// consumes src before returning: CPU memcpy, Metal blit + semaphore wait (shared +// buffers are a memcpy), CUDA memcpyAsync + cudaStreamSynchronize, Vulkan write with +// waitForFences. The src is fully read when the call returns, and nothing reads a +// tensor's src again afterwards. +// +// Only whole pages fully inside BOTH [src, src+nbytes) and the file mapping are +// released: start rounds up and end rounds down, so a page shared with an adjacent +// (possibly not-yet-copied) tensor survives, and a src not contained in the mapping -- +// a heap staging buffer (adapter merge, the f32 and pre-permute loaders) or a stack +// scalar -- is skipped outright. The containment test is unreachable for correctly +// recorded copies (wctx_push_file_copy records in-mapping sources, and wctx_alloc +// gates on file_src), and that is the point: it keeps a future mis-recording a +// harmless no-op instead of a destructive call on the wrong memory. +// +// failed_bytes accumulates the ranges the kernel refused to release, so wctx_alloc +// can report the shortfall for THIS load -- a persistent failure must not be +// invisible just because its first occurrence already warned. +// The strerror detail prints once per process -- inline function-local static, so one +// flag per linked image, and an atomic exchange, so concurrent loads cannot race it. +// Every recurrence is still visible per load through the failed-bytes summary. +inline void wctx_warn_release_failed_once(const char * err) { + static std::atomic warned{ false }; + if (warned.exchange(true)) { + return; + } + fprintf(stderr, + "[WeightCtx] WARNING: %s of a staged tensor's pages failed (%s); page release is not " + "taking effect\n", + wctx_release_op, err); +} + +static size_t wctx_unmap_file_pages(const void * src, + size_t nbytes, + const void * file_base, + size_t file_len, + size_t * failed_bytes) { + const long ps = sysconf(_SC_PAGESIZE); // nanoseconds against a multi-GB copy; no caching + if (ps <= 0) { + // No page size means no release can run at all: count it as failed so the + // per-load summary says so instead of reporting healthy slack. + *failed_bytes += nbytes; + return 0; + } + const uintptr_t page = (uintptr_t) ps; + const uintptr_t s = (uintptr_t) src; + const uintptr_t e = s + nbytes; + const uintptr_t fb = (uintptr_t) file_base; + const uintptr_t fe = fb + file_len; + if (s < fb || e > fe) { + return 0; // not (fully) inside the file mapping -- a staging/scalar src + } + const uintptr_t start = (s + page - 1) & ~(page - 1); // first whole page at/after src + const uintptr_t end = e & ~(page - 1); // last page boundary at/before src+nbytes + if (end <= start) { + return 0; // tensor spans less than one whole interior page + } + if (!wctx_release_pages((void *) start, (size_t) (end - start))) { + // The errno detail once per process; the per-load summary below makes every + // later failure visible regardless. + wctx_warn_release_failed_once(strerror(errno)); + *failed_bytes += (size_t) (end - start); + return 0; + } + return (size_t) (end - start); +} +#else +// No release mechanism on this platform (Windows has neither munmap nor madvise for +// file mappings): the staged pages simply stay until gf_close. +static size_t wctx_unmap_file_pages(const void *, size_t, const void *, size_t, size_t *) { + return 0; +} +#endif + static bool wctx_alloc(WeightCtx * wctx, ggml_backend_t backend) { + // The hatch is decided once per load, here -- not inside the release helper -- so + // the off switch is one decision a second caller cannot forget to re-apply, and + // it rules out the whole mechanism: with release off, overlapping copies are + // harmless duplicate writes, so the sweep below is skipped too. + const bool disabled = wctx_page_unmap_disabled(); + // Enforce the PendingCopy overlap invariant (see the struct comment) BEFORE + // anything is allocated or released: a duplicated file range would fault the + // second copy's read once the first release has run, and failing only after the + // buffer was committed would leave wctx_alloc's failure path holding a live + // multi-GB allocation with pending still armed. Unconditional when release is + // on -- shipped builds are Release, so a NDEBUG-gated check would guard nothing + // that ships -- and cheap: O(n^2) pointer compares over a few hundred entries, + // nothing next to the multi-GB copies it runs between. Same-mapping only: two + // independent GGUFs can never overlap while both are live, and a recycled + // address from a closed mapping is not the same file bytes. + if (!disabled) { + for (size_t i = 0; i < wctx->pending.size(); i++) { + if (!wctx->pending[i].file_src) { + continue; + } + const uintptr_t as = (uintptr_t) wctx->pending[i].file_src; + const uintptr_t ae = as + wctx->pending[i].file_nbytes; + for (size_t j = i + 1; j < wctx->pending.size(); j++) { + const WeightCtx::PendingCopy & b = wctx->pending[j]; + if (!b.file_src || b.file_base != wctx->pending[i].file_base) { + continue; + } + const uintptr_t bs = (uintptr_t) b.file_src; + const uintptr_t be = bs + b.file_nbytes; + if (as < be && bs < ae) { + fprintf(stderr, + "[WeightCtx] FATAL: two pending copies share file bytes -- tied weights " + "must push one copy\n"); + return false; + } + } + } + } wctx->buffer = ggml_backend_alloc_ctx_tensors(wctx->ctx, backend); if (!wctx->buffer) { - fprintf(stderr, "[WeightCtx] FATAL: failed to allocate backend buffer\n"); + fprintf(stderr, + "[WeightCtx] FATAL: failed to allocate backend buffer (is the model too large for " + "this device?)\n"); return false; } // Mark as weight buffer so ggml_backend_sched assigns ops to the correct // backend based on weight location (avoids fallback through expansion). ggml_backend_buffer_set_usage(wctx->buffer, GGML_BACKEND_BUFFER_USAGE_WEIGHTS); - size_t total = 0; + // The hatch is decided once per load, here -- not inside the release helper -- so + // the off switch is one decision a second caller cannot forget to re-apply. + size_t total = 0; + size_t recorded = 0; // staged bytes with a recorded file range + size_t unmapped = 0; + size_t failed = 0; for (auto & pc : wctx->pending) { ggml_backend_tensor_set(pc.tensor, pc.src, pc.offset, pc.nbytes); total += pc.nbytes; + if (pc.file_src) { + // Release the pages this copy came from -- recorded at push time, so it + // still names the original mapping pages even after an adapter merge + // repointed src at a heap staging buffer. + recorded += pc.file_nbytes; + if (!disabled) { + unmapped += wctx_unmap_file_pages(pc.file_src, pc.file_nbytes, pc.file_base, pc.file_len, &failed); + } + } + } + // One render, one print: the parenthetical names the state -- deliberate off, + // released X of Y, or nothing -- so a future edit touches a single fprintf. + char note[128]; +#ifdef ACE_HAS_PAGE_RELEASE + if (disabled) { + snprintf(note, sizeof(note), " (page release disabled by ACE_NO_PAGE_UNMAP)"); + } else { + // The gap between X and Y is boundary pages shared between adjacent tensors, + // sub-page tensors, and every copy made from heap staging (converters) -- all + // of it releases at gf_close. The reader sees the residual and its scale + // instead of guessing what 0.0 MB means for a module of small tensors. + snprintf(note, sizeof(note), " (unmapped %.1f of %.1f MB of staged file pages)", + (float) unmapped / (1024 * 1024), (float) recorded / (1024 * 1024)); + if (failed > 0) { + fprintf(stderr, + "[WeightCtx] WARNING: %.1f MB of staged pages could not be released this load; they " + "stay resident until gf_close\n", + (float) failed / (1024 * 1024)); + } } - fprintf(stderr, "[WeightCtx] Loaded %zu tensors, %.1f MB into backend\n", wctx->pending.size(), - (float) total / (1024 * 1024)); +#else + note[0] = '\0'; // no mechanism on this platform: nothing to report + (void) recorded; // still accumulated for symmetry; nothing to print them against + (void) unmapped; +#endif + fprintf(stderr, "[WeightCtx] Loaded %zu tensors, %.1f MB into backend%s\n", wctx->pending.size(), + (float) total / (1024 * 1024), note); wctx->pending.clear(); wctx->staging.clear(); return true;