fix(llm): pin the model load mode instead of inheriting upstream defaults - #1492
Open
msluszniak wants to merge 1 commit into
Open
msluszniak wants to merge 1 commit into
msluszniak wants to merge 1 commit into
Conversation
barhanc
approved these changes
Sep 24, 2026
msluszniak
force-pushed
the
@ms/llm-mmap-load
branch
from
September 24, 2026 14:02
31bc105 to
e91275f
Compare
…ults Neither factory was called with a load mode, so both inherited upstream's, and the two defaults are wrong in different ways. The multimodal one defaults to LoadMode::File, which reads the whole .pte into a heap buffer: with gemma4_e2b_mlx_int4.pte that walks phys_footprint past 3.3 GB and jetsam kills the app during load. The text one defaults to MmapUseMlockIgnoreErrors, which leaves footprint alone but wires the model resident: resident_size reads 3135-3282 MB against 355-475 MB for plain Mmap. That is the number DeviceInfo.getUsedMemory() and every other memory API reachable from JS reports, so the new API appeared to use 6x the memory of the legacy one on the same model. Pin Mmap for both, which is what the legacy binding has always passed.
msluszniak
force-pushed
the
@ms/llm-mmap-load
branch
from
September 24, 2026 14:05
e91275f to
0c45c0a
Compare
msluszniak
changed the title
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
2 participants
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.
Description
Neither factory was called with a load mode, so both inherited upstream's, and the two defaults are wrong in different ways.
create_multimodal_runnerdefaults toLoadMode::File, which reads the whole.pteinto a heap buffer. Withgemma4_e2b_mlx_int4.pte(2.9 GB) that walksphys_footprintup in ~93 MB steps to 3296 MB and jetsam kills the app during load.create_text_llm_runnerdefaults toMmapUseMlockIgnoreErrors, which leaves footprint alone but wires the model resident. That is the numberDeviceInfo.getUsedMemory()and every other memory API reachable from JS reports, so the new API looked like it used 6x the memory of the legacy one on the same model.Pin
Mmapfor both, which is what the legacy binding has always passed.Introduces a breaking change?
Type of change
Tested on
Testing instructions
iPhone 16 Release, gemma-4-e2b-mlx, after
load():FileMmapUseMlockIgnoreErrors(old text default)Mmap(this PR, and legacy)Related issues
#1489
Checklist
Additional notes
Neither mode changes what jetsam sees on the text path, so this is not the fix for #1489's prefill growth. It is what makes the new API's reported memory match the legacy API's.