fix: map mmapped weights through Metal buffers instead of CPU buffers - #2037
Merged
Merged
Conversation
With mmap enabled on Metal, params that live on the Metal device are mapped from the model file (the device can use host memory in place), but the mapping is always wrapped in a CPU buffer. Metal cannot address a CPU buffer and nothing stages these tensors, so graph execution logs "ggml_metal_buffer_get_id: error: tensor ... buffer is nil" for every weight and the output is garbage. Map such tensors through a buffer the compute device creates from the mapped memory (ggml_backend_dev_buffer_from_host_ptr), created on first use and cached per file, and skip reading them in load_tensors like CPU-mapped tensors. Writable mappings, used when LoRAs are merged in place, are not wrapped; those params are loaded as before. CPU-side backends keep the CPU mapping; backends that cannot wrap host memory never mmap params they compute on, so only Metal changes.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
With
--mmapon Metal, every weight fails at graph execution and the output is noise:can_mmap_storage()allows mmap when the compute backend reportsbuffer_from_host_ptr, which Metal does (unified memory), so params that live on the Metal device are mapped straight from the model file. Butprocess_model_files()always wraps the mapping in a CPU buffer (ggml_backend_cpu_buffer_from_ptr). Metal cannot address a CPU buffer, and because the params backend is the compute backend, nothing stages these tensors either.Change
ModelManager::mmap_params()groups the candidates by the device that computes on them: tensors a GPU computes on in place are mapped through that GPU's device, all others keep the CPU mapping.ModelLoader::mmap_tensors()takes that device and maps the file throughggml_backend_dev_buffer_from_host_ptr(), created on first use and cached per file. If the device cannot wrap the file, its tensors are loaded as before.load_tensors()skips tensors mapped through a device buffer, as it already does for CPU-mapped ones.Only Metal changes behavior. Of the backends that report
buffer_from_host_ptr, the others are CPU-side (BLAS wraps host memory in a CPU buffer anyway), and backends that cannot wrap host memory never mmap params they compute on.Verification
Apple M1 Pro (32 GB), macOS 26,
-DSD_METAL=ON -DGGML_METAL_EMBED_LIBRARY=ON, on master2dc7f54:(Qwen Image 2.1 Q4_K_M and Qwen3-VL-8B-Instruct UD-Q4_K_XL from unsloth, bf16 VAE from Comfy-Org.)
buffer is nilerrors/usr/bin/time -l)--mmap--mmap--mmapWith this PR the log shows
mmap: mapped '...' for MTL0for each GGUF file (the bf16 VAE is converted on load, so it is read as before). The mapped weights stay in the page cache as clean, file-backed pages instead of process memory.Left to right: master without
--mmap, master with--mmap, this PR with--mmap.