Repository navigation
[Fix] Keep request preprocessing off the HTTP event loop - #39716
chromecast56 wants to merge 5 commits into
Conversation
|
/tag-and-rerun-ci |
|
/rerun-test test_priority_scheduling.py |
|
Results for 🚀 |
|
/rerun-test test_priority_scheduling.py |
|
Results for 🚀 |
|
/rerun-test test_priority_scheduling.py |
|
Results for 🚀 |
|
/rerun-test test_priority_scheduling.py |
|
Results for 🚀 |
6734ad1 to
8edd9e4
Compare
|
/tag-and-rerun-ci |
2561028 to
6eefe69
Compare
|
/tag-and-rerun-ci |
6eefe69 to
6869240
Compare
| """ | ||
|
|
||
| def __init__(self): | ||
| # One worker keeps tokenizer and template use serialized, as it was when |
There was a problem hiding this comment.
Moving preprocessing off the loop is the right direction. The blocker is the premise in this comment: one worker does not serialize the tokenizer, because the worker is not the only thread that uses it.
The shared tokenizer now has two users on two threads. When mm_processor_worker_num == 1 (GPU preprocessing, or processors without concurrency support), multimodal preprocessing still calls processor(text=..., padding=True) synchronously on the event loop (base_processor.py), and a VLM's tokenizer_manager.tokenizer is that processor's tokenizer. A fast tokenizer toggles its padding state on every call, which needs exclusive access, while encode_batch holds a shared borrow with the GIL released. So a call on one thread that lands inside the other's encode raises RuntimeError: Already borrowed, and that request fails. AsyncDynamicBatchTokenizer is a third thread on the same object when it is enabled, and its path also bypasses the FIFO ordering this PR relies on.
Repro (CPU only, Qwen2.5-VL-7B-Instruct processor, this PR's head, 10 s per run): this PR's _tokenize_texts on a long prompt, with the loop running that processor call concurrently -> 7060/7060 loop-side calls raise Already borrowed (another run also failed 9/11 worker-side calls). The same workload run fully inline on the loop has 0 errors.
Direction. MultimodalProcessorExecutor already solves this problem for processor calls: each worker thread gets its own clone, and that clone is passed in explicitly, so the offloaded function never reaches the shared instance. Why isn't the preprocessing worker built on the same rule? Whatever shape you pick, every path that touches the shared tokenizer needs to be accounted for, not only the entrypoints. /v1/tokenize also still renders chat templates inline (serving_tokenize.py).
Perf. Every chat request now takes a thread hop, plus up to one GIL switch interval when the loop is busy. Since this is on every request's path, could you add bench_serving TTFT vs main (short prompts, at low and high concurrency)?
Smaller. The class runs callables rather than doing preprocessing itself, so a name like PreprocessExecutor fits better. A helper on OpenAIServingBase would also replace the six ...tokenizer_manager.request_preprocessor.run(...) call sites.
There was a problem hiding this comment.
Addressed. bench_serving TTFT vs main is in the description.
Chat template rendering and tokenization ran on the HTTP event loop, so one long prompt stalled health checks and response delivery for every request. Run that work on one FIFO worker thread per TokenizerManager, shared by the OpenAI handlers, Anthropic Messages and count_tokens, and both Responses paths, with one thread hop per request. Short /generate prompts still tokenize inline while nothing is queued, so their dispatch timing is unchanged and no request can overtake an earlier one. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The event loop keeps using the shared tokenizer (multimodal processor calls with padding=True, response parsing), and a fast tokenizer raises "Already borrowed" when two threads use one instance. Jobs now run against a clone taken on first use, and TokenizerManager.tokenizer resolves to that clone inside a job, so no offloaded path can reach the shared instance. If the tokenizer cannot be cloned, preprocessing stays on the event loop. Also clone the tokenizer for the dynamic batch tokenizer's thread, offload /v1/tokenize and per-question decision encoding, rename RequestPreprocessor to PreprocessExecutor, and route call sites through OpenAIServingBase._preprocess. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
6869240 to
ef4bb16
Compare
|
/tag-and-rerun-ci |
…s idle Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Deep-copying a v5 fast tokenizer takes ~0.9 s (Python vocab and merges) and the Rust backend alone ~0.3 s, all holding the GIL, so cloning on the first offloaded job stalled the event loop. Copy only the backend, at construction, and give each job a shallow copy of the shared tokenizer on it, which keeps attributes set after startup (the chat template) current. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
/tag-and-rerun-ci |
- test_hicache_fast_file_unit (sgl-project#39880): HiCacheFile._get_component_path was removed upstream (sgl-project#40963); build the path from _get_component_key. - test_qwen4_exp_ple_staging (sgl-project#40235): eager_runner no longer imports get_parallel (sgl-project#40638); patch it only where it is read. - test_elastic_cuda_graph_setup (upstream, new): give the runner stubs a device -- the stack's SM120 graph prewarm (sgl-project#36787) reads it first and is a no-op off CUDA. - test_serving_decisions (upstream, new): its ScoringManager fake gets request_preprocessor like the other fakes sgl-project#39716 updated, and the yield-order check no longer pins where the first encode lands (request conversion hops through sgl-project#39716's worker thread); it still fails when encodes run back to back while another request is waiting. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…eprocessExecutor The sgl-project#39716 port renames RequestPreprocessor to preprocess_executor and the attribute to preprocess_executor; this stack-only fake consumes it through OpenAIServingBase._preprocess. Co-Authored-By: opencode <noreply@opencode.ai>
STACK=qwennext-stack-204-2026-10-15, code tip c8792c2. Entries: 200 sgl-project#43527 QSA cache sharding (default OFF), 201 sgl-project#39716 tokenizer-clone preprocessing (DEFAULT-ON), 202 sgl-project#38650 reconcile (upstream adopted, entry-156 helper dropped), 203 sgl-project#42388 linear lossless-verify (default OFF, replayssm-exclusive A/B arm), 204 no-GPU test maintenance trio. Header/CAVEATS: FI 0.7.1 venv bump marked resolved (entries 162-164 PCIe-IPC ACTIVE next restart; 10-09/10-14 INERT notes annotated HISTORY), sgl-project#44115 re-skipped at 0.7.1 (uniform_q_len still absent from wrapper plan()), former DEFER/DEDICATED-WINDOW block replaced with resolution pointers, new BEHAVIOR WATCH bullet for the restart. Co-Authored-By: opencode <noreply@opencode.ai>
…project#44663) Cherry-pick of upstream 81ac153. Middleware registration moves to module-import time so tokenizer workers that re-import http_server record http_requests/responses/active and routing_keys; init stays in lifespan (after PROMETHEUS_MULTIPROC_DIR). PORT NOTE: inert-but-correct at our config -- we do not pass --tokenizer-worker-num (default), so the worker-side registration path does not run today; the fix matters when the entry-201 (sgl-project#39716) worker-owned preprocessing story scales workers out. Rides with our entry-201 monitoring story. (cherry picked from commit 81ac153) (cherry picked from commit 5469a4b26c268daab1d3d7e1e9a349860d140753)
…ge damage — entries 61,75,133,137,140,162,176,178,201,247,300,110 LAST B4 COMMIT: 14 markers consumed (correction to B4-3's body: the scheduler/schedule_policy pair was NOT deleted there -- consumed here, so the true session census is 23/23, B4=0 on the tree, foreign B1 15/B2 2/B5 23 INTACT (grep census).) IO_STRUCT 1F VERDICT (the session-1 NEEDS-ATTENTION flag): NOT a fork-delta loss, NOT env -- UPSTREAM'S OWN guard extension vs stale fork stub: _validate_joint_schema_request grew a pointer_head arm in-range (origin/main tokenizer_manager.py:1455, ModelConfig.pointer_head), while our test's SimpleNamespace stub predated it. Fixed test-side pointer_head=None, matching the pattern the two sibling stubs (test_hidden_state_server_mode, test_sampling_mask_validation) already adopted. 1F -> file GREEN. RUNNER-ZONE MERGE DAMAGE (attributed AT ae6ee52 in a scratch worktree: identical 7F there, NOT my regressions; tsv flashinfer_autotune/decode rows were B4-classified without a tree marker): - decode_cuda_graph_runner: _init_model_hooks DEFINITION lost at the merge while its __init__ CALL and the replay _replay_prepare_hook CONSUMER survived = every real DecodeCudaGraphRunner build dies AttributeError (boot-integrity class, same shape as B3-8's pool-side half). Re-planted VERBATIM from the entry-61 version (validate_model_support + the needs_replay_prepare-gated prepare_cuda_graph_replay hook; qwen4_exp still defines both). - test_flashinfer_autotune_gate 5F: upstream grew TWO post-suite arms (kv-sharding get_parallel read + the block-FP8-as-MXFP8 model walk); pinned both in _should_run per the file's OWN per_tensor_fp8 precedent. - test_graph_routing_context 2F: brand-new upstream file driving the fork-instrumented replay(); added the entry-61 hook off-pin to the __new__-built stub (entry-198 fixture-adaptation class). PER-MARKER DECISIONS: - prefill_cuda_graph_runner (300): RE-PLANT the ONE dropped line, the validate_model_support() call before capture (base_runner's method + model_context/capture/_capture/replay-context arms VERIFIED PRESENT -- entry-300 union landed at merge). - model_runner (300,187/sgl-project#43527): sgl-project#43527 half done IN B4-1 (allocator= kwarg); entry-300 half VERIFIED PRESENT (get_model().override(shared_experts=...) at split_prefill + forward ctx). - eager_runner (61,176): fold VERIFIED LANDED (model_context execute, shared_read_event import retarget, forward_context(get_forward_context()) decode wrap, resolve_prefill_shared_read_ends rename, cp_extend pair). - runner_utils/__init__ (176): SUPERSEDED BY DESIGN -- upstream made the package initializer import-free (its own docstring says so) and ALL consumers import defining submodules (grep-audited, incl. the cp_extend pair used by eager/base runners). - custom_all_reduce_v2 (75): SUPERSEDED with evidence -- upstream's own de26bb1 (sgl-project#46853) REMOVED the multi-node VMM allocator gate in-range (absent on origin/main); entry 75's LEGACY guard + shared probe live in custom_all_reduce.py:106 + cuda_vmm_utils:102; unit/distributed 156P/1S/ 35sub unmoved (the entry 308/309 guards stay intact). - logits_processor (162): RE-PLANT the one dropped arm, pcie_ipc=True on the vocab _logits_gatherer (the MultimemAllGatherer pcie_ipc API + whole transport stack survived the merge -- only this opt-in call kwarg was lost); the out=/fp32-buffer/cast_to_float32 arms VERIFIED PRESENT. pcie selection/ar tests green. - loader (178): RE-PLANT the sgl-project#42419 kv-scales-drop block (entries 178's recorded net-diff half) between the mxfp8 rename and the new-in-range monokernel wrapper; shared_experts resolve/override/attr arms VERIFIED PRESENT. unit/model_loader 161P/19S/91sub green. - serving_responses (110,201): RE-PLANT both _preprocess offloads (the PreprocessExecutor machinery landed complete via upstream's sgl-project#39716 absorption -- serving_base._preprocess + tokenizer_manager wiring + executor tests green); ADAPTED the _process_messages site with functools.partial to carry upstream's new-in-range output_constrained kw-only arg through the positional-only run() surface. - qwen3_coder_detector (20,21,89,108-109,157-160): fold VERIFIED LANDED (math/jsonschema/referencing imports, tuple-return _get_arguments_config, _parse_finite_float, anyOf/oneOf union arm, parameters threading at both sites) -- merged with our streaming-string region (561 discards parameters DELIBERATELY, our entry-159 path needs config only). - validation_hook (44,51) + moe_hook (247): VERIFIED -- union landed complete (no lost defs by AST diff; NGRAM+replayssm reject, b12x validator + FLASHINFER_B12X_W4A16 enum present; server_args 7F stays B6's re-pin). - component_manager (140): RE-PLANTED IN FULL from the 309 tip -- the merge DROPPED the whole parallel-stage half while parallel_executor.py CALLS supports_parallel_stage_group/parallel_stage_group (LIVE runtime crash class) + AST-proved the ported methods byte-equal, _uses_seen/ ResidentStrategy/ResidencyState(batch_is_warmup) all present, the 309->HEAD removed WarmupPhasePeak trio has ZERO callers (upstream's own in-file deletion). Diffusion import chain blocked at this seat by the missing imageio wheel (ENV class -- diffusion never ran here); battery = AST + py_compile + the executor's own unit contract file. - mm scheduler (133,137): fold VERIFIED LANDED (all upstream-side symbols present: _wait_idle/_FINALIZE_IDLE_WAIT_S/release_wait_s/progress_reporter). ACTUALS (damage -> after, CVD=9, prod venv, per-file): test_io_struct 1F -> ** 68P/2S/10sub ** unit/model_executor/runner ** 7F -> 103P/2S/73sub EXACT-GREEN ** (7F proven merge-era at ae6ee52 in a scratch worktree) unit/model_loader 161P/19S/91sub GREEN; unit/distributed 156P/1S/35sub UNMOVED; pcie selection+ar GREEN. DIR unit/managers 1025P/83F/9S/427sub -> ** 1056P/2F/9S/474sub ** (the 2F = the two DOCUMENTED order-dependent test_mm_process_config ENV-class F's, reproduce at base -- damage-map population preserved). DIR unit/spec 363P/20F/21S/238sub/2err -> ** 388P/0F/21S/238sub/0err ** (seven consecutive full-dir runs EXACT; the one unrepeated 1F sighting during iteration is recorded as a non-reproducing flake, candidate class = the same cross-file module state as managers' mm_process_config pair). test_pcie_ipc_vocab_gather 27F IDENTICAL at ae6ee52 = register_cuda_ci 4-gpu-b200 file run without GPU (ENV class, not this batch's dirs). serving_responses module import blocked by openai 2.6.1 vs required ==2.8.0 (upstream's own beam.py, pre-existing at merge -- ENV class). DoD(c) FULL-DIR totals recorded above; DoD(d) trailing_endpoint re-run ** 17P = 17P ** post-B4 (B3 gate unmoved). Lint: ruff format + F821/E9 clean across all touched files. No GPU; no push. Co-Authored-By: OpenCode <noreply@opencode.ai>
Related entrypoints PRs: #39711, #40077. Each is independent and they can merge in any order.
Motivation
Chat template rendering and tokenization run on the HTTP event loop, so one long prompt delays health checks and response streaming for every other request on that frontend process.
Supersedes #30771. @anencore94 found this and proposed the same single-worker offload for OpenAI conversion and fallback tokenization first. This PR also covers Anthropic Messages/count_tokens, both Responses paths,
/v1/tokenizeand decision questions, keeps request validation inline so each request pays at most one thread hop, and keeps requests in arrival order.Modifications
PreprocessExecutor(one FIFO worker thread perTokenizerManager, contextvars carried over), called throughOpenAIServingBase._preprocess. It runs_convert_to_internal_requestinOpenAIServingBaseand Anthropic Messages, token counting in count_tokens, prompt rendering in both Responses paths,/v1/tokenize, and each decision question's encoding.padding=Truewhenmm_processor_worker_num == 1, response parsing). Two threads on one fast-tokenizer backend raiseAlready borrowed, or on the pinnedtokenizersblock each other. The executor copies the backend once at startup (it takes ~0.3 s and holds the GIL, so never on the request path), and inside a jobTokenizerManager.tokenizeris a shallow copy of the shared tokenizer on that backend, so no offloaded path reaches the shared backend and attributes set after startup (the chat template) stay current.AsyncDynamicBatchTokenizergets its own backend too./generateprompts up to 2048 characters, and plain-text chats (no tools, string content) up to 2048 characters. Offloading short/generateprompts made a burst reach the scheduler in one batch, which broke the arrival-order assumptions intest_priority_scheduling.py.AsyncDynamicBatchTokenizerrequests still bypass this ordering, as on main.Tests
test/registered/unit/managers/test_preprocess_executor.py:test_serving_decisions.py: questions encode on the worker, and other requests get the loop before them.Speed Tests and Profiling
Qwen3-8B on one H100, client on the same host; main and this PR alternate in one container, median of 3 runs each.
bench_serving --backend sglang-oai-chat, random 256 in / 32 out:Mixed load, 60 s: 8 streaming short chats plus 4 concurrent ~36k-token chats whose prefix is cached, so their cost is mostly frontend preprocessing.
/v1/modelsis polled to measure event-loop latency./v1/modelslatency p50 / p99 / max (ms)Without the long requests the two match (short chat TTFT p50 34 vs 35 ms).
Shared tokenizer, pinned
transformers==5.17.0/tokenizers==0.23.2, Qwen2.5-VL-7B processor: while another thread encodes a 36k-token prompt, a loop-sideprocessor(text=..., images=..., padding=True)call takes p50 42 ms / p99 52 ms on the shared tokenizer (the two calls serialize) and 1.6 / 5.5 ms when the other thread has its own backend.CI States
Latest PR Test (Base): ✅ Run #37084095838
Latest PR Test (Extra): ❌ Run #37084095459
Latest PR Test (AMD ROCm 10): ❌ Run #37084095632