Make BPE Pre-tokenizer Thread-Safe by Using Per-call Cursors - #1104
Open
Sayan Shaw (sayanshaw24) wants to merge 3 commits into
Open
Make BPE Pre-tokenizer Thread-Safe by Using Per-call Cursors#1104Sayan Shaw (sayanshaw24) wants to merge 3 commits into
Sayan Shaw (sayanshaw24) wants to merge 3 commits into
Conversation
added 2 commits
August 19, 2026 15:05
…sions into sayanshaw/tokenizer-thread-safety
Contributor
There was a problem hiding this comment.
Pull request overview
This PR addresses a concurrency bug in the BPE tokenizer pre-tokenization path by ensuring each Tokenize()/SpmTokenize() call uses its own PreTokenizerWithRegEx cursor state while still reusing cached compiled patterns.
Changes:
- Introduces
PreTokenizerWithRegEx::CreateCursor()to create per-call cursor instances while sharing compiled regex/matchers. - Switches pre-tokenizer compilation initialization to
std::call_onceto avoid racy lazy initialization ofcached_splitters_. - Updates the BPE kernel tokenization paths to use per-call cursor instances (including sequence pre-tokenizers).
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| operators/tokenizer/bpe_utils.hpp | Adds CreateCursor() and switches fallback regex ownership to shared_ptr for sharing across cursors. |
| operators/tokenizer/bpe_kernels.h | Adds <mutex> and a std::once_flag to support thread-safe pre-tokenizer compilation. |
| operators/tokenizer/bpe_kernels.cc | Uses std::call_once and per-call cursors to make regex pre-tokenization thread-safe. |
Suppressed comments (1)
operators/tokenizer/bpe_kernels.cc:656
- Same issue as in
Tokenize():std::call_oncewill still run the lambda once even ifcached_splitters_was already compiled during model load, causing a redundantCompilePreTokenizer()on the firstSpmTokenize()call. Guard the body so the once-call becomes a no-op whencached_splitters_is already initialized.
std::call_once(compile_pretokenizer_flag_, [this]() {
const_cast<KernelBpeTokenizer*>(this)->CompilePreTokenizer();
});
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Sayan Shaw (sayanshaw24)
enabled auto-merge (squash)
August 20, 2026 03:51
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.
Make BPE pre-tokenizer thread-safe by using per-call cursors
Problem
PR #1068 optimized BPE tokenization by caching compiled
PreTokenizerWithRegExinstances as member variables (cached_splitters_). This avoided re-compiling regex patterns on everyTokenize()call.However,
PreTokenizerWithRegExhas mutable cursor state (m_text,m_last_char,m_utf8_text) that gets overwritten on everySet()+GetNextToken()call. When multiple threads callTokenize()on the same tokenizer instance concurrently, they race on this shared cursor, causing corrupted token output.This was observed as flaky test failures in downstream consumers (microsoft/foundry-local#1008).
Root Cause
reg_splitter.m_textSet(),GetNextToken()reg_splitter.m_last_charTryMatch()reg_splitter.m_utf8_textSet()cached_splitters_pointerconst_castinitFix
operators/tokenizer/bpe_utils.hppCreateCursor()method that returns a lightweightPreTokenizerWithRegExsharing compiled state but with independent cursor state.activated_matchers_toshared_ptr<vector<RegexMatchFunc>>— avoids heap-allocating a copy of the matchers vector on everyCreateCursor()call.fallback_patterns_toshared_ptr<const std::regex>— enforces immutability of compiled patterns and enables sharing across cursors.operators/tokenizer/bpe_kernels.ccTokenize()andSpmTokenize(), replace direct use ofcached_splitters_->reg_splitterwithauto splitter = cached_splitters_->reg_splitter.CreateCursor()— each call gets its own cursor on the stack.seq_splittergets a local cursor.const_castlazy init withstd::call_onceto eliminate the TOCTOU race oncached_splitters_initialization.CompilePreTokenizer()withonce_flagto prevent double-compile when called eagerly at load + lazily viacall_once.operators/tokenizer/bpe_kernels.hstd::once_flag compile_pretokenizer_flag_member forstd::call_once.Performance Impact
None. The compiled regex patterns and matcher function vectors (the expensive parts) are shared via
shared_ptr— no heap allocations inCreateCursor(). Each cursor is ~40 bytes of stack state (astring_view, achar32_t, an emptystring, and twoshared_ptrcopies).Verified with
test_performance.py— GPT-2 long_english: 16.8 MB/s (vs 15.6 MB/s before fix). No regression.Testing
All existing tokenizer tests pass.