Fix memory issues when installing models on Windows #8652
+45
−13
Conversation
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
Wrap gguf.GGUFReader and then use a context manager to load memory-mapped GGUF files, so that they will automatically close properly when no longer needed. Should prevent the 'file in use in another process' errors on Windows.
Additional check for cached state_dict as path is now optional - should solve model manager 'missing' this and the resultant memory errors.
github-actions
bot
added
python
|
I'm sorry but I can't seem to appease ruff here, even when configuring mine with this repo's pyproject.toml. |
gogurtenjoyer
changed the title
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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
Windows treats file handles differently than MacOS and Linux - Model Manager v3 has exposed some issues here, so here's a few patches to solve this. I wrote more but then the power went out, and I'm not rewriting all of that, but I wanted to make sure to thank @skunkworxdark for catching and fixing a serious memory issue.
Related Issues / Discussions
Closes:
#8644
#8636
#8628
#8627
QA Instructions
To test, import various models from Starter Models and elsewhere: GGUF files, Safetensors files, particularly large ones, and Diffusers multi-file models. This fix is specifically for Windows, but the changes should benefit every OS and not break anything on MacOS and Linux. Tested on MacOS and Windows so far.
Merge Plan
Changes the GGUF loader, and import_state_dict in model_on_disk.py - shouldn't conflict with anything; mostly small/remote changes.
Checklist
What's Newcopy (if doing a release after this PR)