Detach tokenizer storage from model mmap - #942
Open
riccardomenegazzo wants to merge 1 commit into
Open
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
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.
Related to #922.
DS4 currently stores vocabulary tokens and BPE merge keys as borrowed
ds4_strspans into the GGUF mmap. This leaves tokenization and token decoding able to fault model storage long after model loading has completed.This change detaches only tokenizer string metadata from the model mapping:
ds4_vocab;token_to_idandmerge_rankexclusively from owned spans;vocab_free().Tensor data remains mmap-backed. Metal, CUDA, ROCm, distributed inference and SSD expert streaming retain their existing storage semantics.
Regression test
tests/test_vocab_storage.ccreates synthetic GGUF-style token and merge arrays inside an anonymous mapping, loads the tokenizer, verifies that every retained span is detached, and then removes the source mapping withmunmap().After the mapping no longer exists, the test exercises token lookup, BPE merge lookup and emission, token byte access, and the actual
ds4_token_text()literal-special path from #922.Validation
Run on Linux x86_64:
make -B ds4.omake -j2 cpu./tests/test_vocab_storage./tests/test_engine_mgpu_placement- 109/109 checks passed./tests/test_sampling- passedThe original 96 GiB external-SSD reproduction and Metal/CUDA/ROCm runtime tests were not available in my environment. The patch does not modify backend or tensor execution code.