cuda: Windows MoE pin budget - layer-subset routing + chunked pinned store - #84
jasonlnheath wants to merge 2 commits into
Conversation
|
The Windows measurements are useful, and smaller registration retries could help #76. This patch needs a few fixes before we can integrate it. Reviewing head
For integration with updated #76 ( Please attach the native Windows commands, allocation logs and sanitizer results mentioned in the description, with completed inference/output comparisons and repeated allocation/free coverage. We still need evidence that the proposed registration change improves the bounded path. The checks above are source review plus an extracted compiler check, not a completed CUDA build or native Windows runtime validation. AI disclosure: Codex assisted with this review and comment under repository-owner direction. |
…store Windows WDDM caps page-locked host memory far below what a full expert-set pin needs (measured on 64 GB RAM: 28 GB aggregate in a bare process, only 2-4 GB total inside a loaded server process), so the expert cache silently falls back to plain CPU buffers on Windows - all-zero counters, no warning, uncached speed. - GGML_CUDA_MOE_PIN_LAYERS=N: route only the first N layers' expert tensors through the cached buffer type so the pinned store fits a budget; the remaining layers compose with --cpu-moe (prepend-order, first-match-wins confirmed). - Windows chunked pin allocator: VirtualAlloc the full range, then cudaHostRegister in adaptive chunks (ladder probes down from 4 GiB). Every chunk must register or the allocation unwinds - a partially-pinned store crashes launch_mm_ids_helper with illegal memory access (confirmed). - Override pattern strings must outlive the loader: a block-scoped std::string dangles its c_str() and the regex silently matches nothing (found the hard way). Blocked on typical 64 GB boxes by the in-process pin ceiling itself, not by this code: a 22-layer subset needs ~24 GiB of pins. On 128 GB boxes both quota classes fit and this should give full cached engagement. The ladder doubles as a diagnostic of what a given box/driver grants. Co-Authored-By: Claude Code <noreply@anthropic.com>
… NO_PINNED - move the win pin free callback and win_chunked inside _WIN32: the callback referenced a Windows-only symbol on all platforms - store registered segments per allocation and walk them in reverse on free and unwind; a process-wide chunk stride skipped registrations when interleaved allocations resolved different sizes - check unregister and VirtualFree returns; on failure keep the VA range and log, never free a registered range - honor GGML_CUDA_NO_PINNED before the Windows pin path - serialize pin and unpin with a mutex (the WDDM quota is process-wide) - validate GGML_CUDA_MOE_PIN_LAYERS (positive integer, clamp 512) and correct the placement log: layers past N keep normal placement - test: interleaved win pin alloc/free cycles in test-moe-cache Assisted-by: Claude Code
343a03b to
0aed729
Compare
|
Thanks for the detailed review. All four source issues are fixed in 0aed729, the branch is rebased onto moe-cache@d30efee88, and native Windows evidence is below. 1. Non-Windows buildThe free callback and the win_chunked flag now live inside the _WIN32 guard; the iface assignment is a default plus a guarded override. Linux never sees a Windows-only symbol. The ubuntu CUDA workflow needs maintainer approval to run for first-time fork contributors and has not started on this push, so your extracted compiler check is the reference - it should now pass. 2. Registration ownershipg_moe_win_pin_chunk is gone. Each allocation now records its registered segments:
Why a registry instead of per-buffer state: ggml_backend_cpu_buffer_from_ptr uses buffer->context as the data base for the shared CPU iface (ggml-backend.cpp:2527), so per-allocation metadata cannot ride on the buffer without a custom iface. Pin and unpin are serialized by a mutex; the WDDM quota is process-wide. Test coverage: test-moe-cache gained interleaved alloc/free cycles (512 MiB + 64 MiB allocated together, freed out of order, 3 cycles) with a --win-pin-only mode. Passes on this rig, and runs clean under compute-sanitizer memcheck (ERROR SUMMARY: 0 errors). 3. GGML_CUDA_NO_PINNEDChecked at the top of the Windows allocator, before VirtualAlloc. With the variable set there are zero registration attempts and the model generates on plain CPU buffers, same as the other paths honoring the variable. 4. Layer placement
RebaseReplayed onto d30efee. The src/llama.cpp hunk is re-expressed on the proc-address buffer-type acquisition from 12a4d1d; nothing else in that block changed. Evidence (Windows 11, RTX 5070 Ti 16 GB, 64 GB RAM, CUDA 13.1, sm_120, Flash-Next UD-Q3_K_XL)
Greedy comparison (--cpu-moe reference vs cache-enabled with pin failure fallback): byte-identical outputs on 3 prompts x 120 tokens, temp 0. The PIN_LAYERS=2 crash is the partial-pin crash class the original commit documented: routing is verified correct in the logs, the subset pins fully, and the fault appears only when the pinned-subset cache path engages at generation. It did not reproduce on the pre-Sep-8 base this patch was developed against, so it most likely interacts with the grouped-decode changes merged since. I have not root-caused it yet; the win-pin allocator itself is memcheck-clean, so I believe the allocator is not the origin. Full-model compute-sanitizer is not feasible on this box: the sanitizer's own allocations push the WDDM pinned quota over during model load (cudaMallocHost OOM under instrumentation). The Sep 7 targeted-repro sanitizer report in lv5-console-for-gen.zip (previously shared on Discord) remains the memcheck evidence for the split-staging stream-memop issue - and note the Windows test suite currently fails at the split-staging sync test (test-moe-cache.cpp:10982) with the same signature. The suite also needs a 32 MB stack on MSVC (default 1 MB overflows in the certificate tests). #76 interactionKept separate as you suggested. I test-drove a smaller-prefix retry in ggml_backend_cuda_moe_pin_sources on 6b0b058:
Patch (retry loop + the two test hooks) is at jasonlnheath@868a29314 on top of 6b0b058 - or I can send the diff directly, whichever you prefer. One contiguous registration per source is preserved; your budget and ownership structures are untouched; layer policy stays out. |
|
Follow-up on the PIN_LAYERS=2 illegal access from my earlier comment - root-caused, and it is not this patch's allocator. What it is not (all verified on the rig, logs available):
What it is: the fault is asynchronous and outside the calling thread's streams. Every main-thread sync point reports clean, then the next CUDA API call (cudaFuncSetAttribute inside launch_mm_ids_helper, serving a CUDA_Host layer's prefill MMQ) returns the sticky illegal access. The crash appears exactly when a pinned expert cache engages during prefill; the same routing with pageable fallback buffers (GGML_CUDA_NO_PINNED=1) generates fine at 8 t/s, and full-set fallback generates fine at 23 t/s. That converges with two things already on record: the Sep 7 sanitizer report (cuStreamWriteValue32_v2 to a host-mapped stage_ready flag in ggml_cuda_moe_cache_prepare_split_staging - stream memops against host-mapped memory are WDDM-restricted) and the Windows test suite failing today at the split-staging sync test (test-moe-cache.cpp:10982) with the same signature. Pageable sources take plain copies and never enter that path; pinned sources do. So: GGML_CUDA_MOE_PIN_LAYERS=2 routes layers 0-1 experts to the cached buffer type, the pin engages (first configuration on a 64 GB box that ever engaged the cache), the first multi-row evaluation splits into staged waves, and the split-staging stream memop faults. The subset routing itself is verified correct in the load logs. Repro: Until the split-staging path gains the device-memory-flag/event-sync treatment discussed for it earlier, the safe Windows story for this patch remains: pin budget + ladder as diagnostics, cache fallback and NO_PINNED paths as the runtime modes. Happy to hand over the instrumented logs (per-op traces, ~490 sync points) if useful for the fix. |
|
Follow-up on this: I introduced fixes based off of this fork in a commit a few days ago. If you'd like to test again and see if this has been removed, that would be great. |
Complements #76 (bounded pin budget) with a Windows-tested layer-subset router and a chunked pinned-store allocator, plus the WDDM measurements behind them.
Problem
On Windows the expert cache's full-set pin silently falls back to plain CPU buffers when the driver refuses the allocation — all-zero
tensors/coveredcounters, no warning, uncached speed. WDDM caps page-locked host memory far below what a full expert-set pin needs.Measured on this rig (RTX 5070 Ti 16GB, 64 GB RAM, CUDA 13.1, Windows 11)
cudaHostRegister~= 28 GB obtainablePageableMemoryAccess=0,HostRegisterSupported=1What this patch adds
GGML_CUDA_MOE_PIN_LAYERSsubset routing — pin a chosen subset of layers when the full expert set cannot fit, instead of nothing. 66/66 tensors claimed on Flash-Next UD-Q3_K_XL. Includes a scope fix: the pattern string must outlive the loader (otherwise a dangling pointer silently drops claims).cudaHostRegisterallocator — ladder of shrinking chunk sizes. Doubles as a diagnostic of what a given box/driver actually grants, turning silent fallback into measurable data.Honest scope
This does not reach the full cached path on 64 GB WDDM — the in-process grant is 2-4 GB versus the ~24 GB the cached path wants. Where it helps: bigger-RAM Windows boxes (128 GB: AnimaLoraStudio measured the ~50% cap; vetchahoo's box pins fine) and anyone diagnosing WDDM grant limits. Possible follow-up: tensor data pointers are 64B- rather than 4KB-aligned —
cudaHostRegisterwants 4K alignment, so alignment fixup may improve grant sizes.The compute-sanitizer report and full env diagnostics were shared on Discord earlier; happy to attach here too.
🤖 Generated with Claude Code