Repository navigation
perf(model-manager): identify GGUF models from the header alone - #9816
Open
Pfannkuchensack wants to merge 1 commit into
Open
Pfannkuchensack wants to merge 1 commit into
Pfannkuchensack wants to merge 1 commit into
Conversation
Build identification's GGUF state dict as meta-backed GGMLTensors from the header instead of copying every tensor. Peak RAM per probe drops from about the file size to megabytes; drop gguf_sd_loader's identification-only mode.
This branch has not been deployed
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.
Summary
Installing a GGUF model no longer reads its tensors into RAM. Identification probes each file with every candidate config, and for GGUF that meant
gguf_sd_loader, which copies every tensor out of the file. Memory peaked at about the file size, plus as much again in the working set. A 23.6 GB transformer therefore needed ~22 GiB of free RAM just to be identified, and a machine with less RAM could not install it at all.Identification reads names, shapes and quantization types, never values, and the GGUF header holds all of them.
ModelOnDisk.load_state_dict()now builds the sameGGMLTensors from the header via the newgguf_header_state_dict, the way the safetensors branch already does withmetatensors:comfy.gguf.orig_shape, comes from the same_logical_shapehelper the full read uses.metadevice.gguf_sd_loader'sq8_cr="ignore"mode existed only for identification and is removed.Before / after, measured on real files (peak private memory of one identification; time with a warm file cache):
What time remains is the whole-file hash.
Related Issues / Discussions
Found while adding LTX-2 GGUF support (#9814, #9734). #9814's LTX-2 docs say installing a GGUF reads the whole file into RAM. Whichever PR merges second drops that sentence.
QA Instructions
Audit. Nothing reads tensor values during identification. The configs read only
.shape,tensor_shape, dtypes and theGGMLTensortype. The otherModelOnDiskusers are the reidentify route (which runs the same identification) and a migration that only reads the file size.Real files. I identified 15 local GGUFs both ways, header and full read, and every file produced the identical config:
The 0.5 GiB cases are encoders, whose tokenizer metadata is parsed either way.
Tests.
orig_shapetensor: same type, same logical and packed shapes, same storage dtype, and meta storage. Mutation probes on the packed shape and onorig_shapeturn it red.ModelOnDisk.load_state_dict()returns meta-backed tensors for a GGUF. Reverting the branch to the full read turns it red.pytest -n 4 tests/backend/quantization tests/backend/model_manager tests/model_identification: 2616 passed, 156 skipped, 1 xfailed.Review
No blocking findings. Resolved:
ModelOnDisk.load_state_dict()returns meta-backed tensors.Q8_CRlayers, which come back as rawI8codes without markers.Left as follow-ups, both outside this change:
WrappedGGUFReader.close()runsgc.collect(), about 0.19 s of a 0.2 s header read, and it frees nothing because the memmap is released by refcount on return.model_util.read_checkpoint_metahas no callers and a comment that no longer matches.Checklist
What's Newcopy (if doing a release after this PR)Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.