fix(frontend): offload prompt tokenization off the async event loop - #11200
Conversation
WalkthroughThe preprocessor's tokenization path is converted from synchronous to asynchronous. ChangesAsync Tokenization Conversion
Estimated code review effort: 2 (Simple) | ~10 minutes Sequence Diagram(s)sequenceDiagram
participant Preprocessor
participant gather_tokens
participant encode_with_timing
participant TokioBlockingPool
participant RequestTracker
Preprocessor->>gather_tokens: await gather_tokens(request)
gather_tokens->>encode_with_timing: await encode_with_timing(prompt)
encode_with_timing->>TokioBlockingPool: spawn_blocking(tokenizer.encode)
TokioBlockingPool-->>encode_with_timing: token ids
encode_with_timing->>RequestTracker: record tokenize latency
encode_with_timing-->>gather_tokens: tokens
gather_tokens-->>Preprocessor: tokens, metadata
Related issues: None referenced. Related PRs: None referenced. Suggested labels: performance, rust, preprocessing Suggested reviewers: None specified. 🐰 A hop, a skip, a tokenizer's dance, 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
lib/llm/src/preprocessor.rs (1)
1694-1712: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsolidate duplicate
encode_with_timingcalls.Both the
has_backend_instance_idand fallback branches now perform the identicalself.encode_with_timing(prompt, tracker).await?followed by the same tuple construction — only the warning log differs. Since both branches were touched to add.await, this is a good opportunity to de-duplicate.♻️ Proposed refactor
- let (tokens_vec, skip_token_annotation) = if let Some(tokens) = - token_data - { + if has_backend_instance_id && token_data.is_none() { + tracing::warn!( + "backend_instance_id provided but no token_data; tokenizing prompt" + ); + } + let (tokens_vec, skip_token_annotation) = if let Some(tokens) = + token_data + { tracing::info!( token_count = tokens.len(), first_tokens = ?&tokens[..std::cmp::min(5, tokens.len())], "[SIDECAR-SKIP-TOKENIZE] Found nvext.token_data — using pre-computed tokens, SKIPPING tokenization" ); (tokens.clone(), true) - } else if has_backend_instance_id { - tracing::warn!( - "backend_instance_id provided but no token_data; tokenizing prompt" - ); - let encoding = self.encode_with_timing(prompt, tracker).await?; - (encoding.token_ids().to_vec(), false) } else { let encoding = self.encode_with_timing(prompt, tracker).await?; (encoding.token_ids().to_vec(), false) };🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/llm/src/preprocessor.rs` around lines 1694 - 1712, The `encode_with_timing` call in `preprocessor.rs` is duplicated across the `has_backend_instance_id` and fallback paths in the `tokens_vec` / `skip_token_annotation` branch. Refactor the `token_data` handling inside `preprocessor.rs` so `self.encode_with_timing(prompt, tracker).await?` and the `(token_ids, false)` tuple are computed once, while preserving the `tracing::warn!` only in the `has_backend_instance_id` case and keeping the `tracing::info!` path for precomputed `token_data`.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@lib/llm/src/preprocessor.rs`:
- Around line 1694-1712: The `encode_with_timing` call in `preprocessor.rs` is
duplicated across the `has_backend_instance_id` and fallback paths in the
`tokens_vec` / `skip_token_annotation` branch. Refactor the `token_data`
handling inside `preprocessor.rs` so `self.encode_with_timing(prompt,
tracker).await?` and the `(token_ids, false)` tuple are computed once, while
preserving the `tracing::warn!` only in the `has_backend_instance_id` case and
keeping the `tracing::info!` path for precomputed `token_data`.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: e61b9d12-3077-4961-a838-d33c62b2b480
📒 Files selected for processing (1)
lib/llm/src/preprocessor.rs
|
lib/bindings/c/src/lib.rs — This call still matches on 🤖 AI FixIn |
53feb96 to
ea58ce6
Compare
ea58ce6 to
127cbda
Compare
ROOT CAUSE of the DSV4 disagg SLO collapse: the frontend generate path tokenized the ~40k-token prompt SYNCHRONOUSLY on the async event loop (gather_tokens -> encode_with_timing -> tokenizer.encode), unlike the embedding path which already offloads via spawn_blocking. At high concurrency this stalls the frontend tokio runtime multi-second (measured: 135 event-loop stalls, max 2.66s), starving the request-plane I/O -> 5s ACK timeout -> CannotConnect -> GEN worker inhibited -> flap/cascade -> throughput collapse (105 -> 14-30 req/s), while GEN/CTX engines stay healthy. Evidence: iter4 C8diag2 stall logs + send->decode 14.6s arrival. Fix: make encode_with_timing async and run tokenizer.encode on the bounded blocking pool (spawn_blocking), mirroring the embedding path; propagate async through gather_tokens and its two callers. Frees the event loop so request-plane I/O is polled -> no false CannotConnect -> no cascade. Signed-off-by: Yuewei Na <nv-yna@users.noreply.github.com>
127cbda to
625ea9b
Compare
What this PR does
Moves prompt tokenization off the frontend's async (tokio) event loop and onto the bounded blocking
thread pool.
Why / the improvement
The generate path tokenizes the prompt synchronously on the async event loop
(
gather_tokens→encode_with_timing→tokenizer.encode), unlike the embedding path(
preprocess_embedding_request), which already offloads encoding viaspawn_blocking. For long promptsunder concurrency, the CPU-heavy BPE encode blocks the runtime for multiple seconds, stalling every other
task that shares it.
Offloading the encode keeps the event loop responsive while a prompt is tokenized, so request latency and
throughput no longer degrade when large prompts are encoded under load. In disaggregated serving this also
stops the stall from starving request-plane I/O (which could otherwise cause healthy workers to be wrongly
marked down).
How
encode_with_timingbecomesasync;tokenizer.encoderuns insidetokio::task::spawn_blocking.Arcis cloned so the closure is'static + Send.asyncis propagated up throughgather_tokensand its call sites. The one synchronous FFI consumer(
lib/bindings/cEPP tokenize path) bridges the now-async call via the crate's existingruntime.secondary().block_on(...).Encoding; null-byte stripping ispreserved.
Scope
2 files —
lib/llm/src/preprocessor.rsandlib/bindings/c/src/lib.rs(+24/−12).Testing
cargo fmt -- --checkandcargo clippy --no-deps --all-targets -- -D warnings(full workspace) both passon top of
main.