Repository navigation
Serve Clef on the Rust runtime instead of ThreadingHTTPServer - #182
Draft
hsliuustc0106 wants to merge 1 commit into
Draft
hsliuustc0106 wants to merge 1 commit into
hsliuustc0106 wants to merge 1 commit into
Conversation
ThreadingHTTPServer scored each request on a fresh thread, so the first CUDA touch on that thread paid the context-init tax. omni-clef admits one request at a time through SerialScheduler and forwards the body to a single Python process whose main thread owns the pinned joint-schema executor. The joint head stays in the checkpoint module. Co-authored-by: Hongsheng Liu <liuhongsheng4@huawei.com>
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
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.
Required human contributor answers
Answer every question before implementation and PR submission, including drafts.
Reuse explicit, still-applicable human answers with a traceable source reference;
ask the human for anything missing and wait. An agent must not invent answers.
Blank fields, placeholders, or an agent-written rationale do not satisfy this gate.
Share only information and source links appropriate for this PR's audience.
What outcome are you trying to achieve?
Answer: Move the default Clef-Flash serve path off Python
ThreadingHTTPServerand onto the shared Rust runtime, while keeping/v1/systemoneand/healthbehavior correct. Source: the task that opened this branch (draft PR only; do not mark ready).What is the highest-leverage variable for that outcome?
Answer: Where the first CUDA touch happens. On the L20X-labeled host at main
a41e379, HTTP serve was about 162–175 ms and the same executor on a main or persistent thread was about 48–65 ms. The difference is the first CUDA touch on a fresh HTTP thread, about 110 ms. Public231 C1 on that host was about 167/214 ms versus vLLM about 32/83 ms. Source: the same task. Not re-measured in this PR.How will this work move that variable rather than create busywork?
Answer: Admit requests on
omni-runtime'sSerialSchedulerand score them on one persistent executor thread, reusing the existing Clef executor. That removes the ephemeral-thread CUDA tax. A full native joint-head port is a later slice; it is not required to move this variable. Source: the same task (Hongsheng chose the Rust migration, not a Python-only dedicated-thread patch).Who is the customer or user, and what do they actually need?
Answer: Operators serving Clef-Flash. They need the current request and response envelope, and a default path that does not pay the ephemeral-thread CUDA tax. Lab L20X smoke and Public231 C1 of this server are a separate follow-up. Source: the same task.
After answering the four questions: is a PR needed at all to achieve this outcome?
Human decision and rationale (or explicit source reference): Yes. The task asks for a draft PR on InferMatrixLab/system1-omni and says not to mark it ready, merge, or post review comments.
For a bug fix: link actual observed failure evidence.
Include the observed result of a minimal reproduction, failing regression test,
or relevant trace. Label untested diagnoses as unverified. For a non-bugfix,
state why this is not applicable.
Evidence: Operator report in the task, L20X-labeled host, main
a41e379. Public231 C1: omni Clef about 167/214 ms versus vLLM about 32/83 ms. HTTP about 162–175 ms versus main-thread or persistent-thread serve about 48–65 ms. This environment did not re-run that measurement. The regression tests added here prove the new default path does not serve CUDA work on a freshThreadingHTTPServerthread; they do not measure latency.Purpose
frontend.clefusedThreadingHTTPServer, so each request ran on a new thread. PyTorch CUDA state is thread-local, and the first touch on that thread cost about 110 ms. StartLux already avoids that by serving through Rust. This PR makes the same shape the default for Clef.Design: hybrid first slice, not a native joint head. The joint schema head is the checkpoint's
joint_schema_model.pyplusjoint_head.safetensors. The shared Qwen CUDA executor scores a different head and is not used. Reimplementing that head is a separate milestone. This slice is the largest change that removes the thread tax without leaving a second HTTP server as the default:omni-clefis the HTTP server. It speaksGET /healthandPOST /v1/systemone.omni_runtime::SerialScheduler(same FIFO queue as StartLux and Laya). A second request waits. The previous handler returned HTTP 503 while the lock was held.python -m models.clef.worker, loads the pinned snapshot, warms up, and scores every request on its main thread. That is the thread that initialized CUDA.python -m frontend.clefexecsomni-clef. It does not bind a port and it does not fall back to a Python HTTP server.The port opens only after the executor's startup forward returns. Response JSON from the executor is forwarded unchanged, including answer order. Transport errors keep the previous
{"detail": ...}text for the cases the old handler tested (400, 404, 408, 411, 413, 415, 422, 500). A negativeContent-Lengthis rejected by hyper as HTTP 400 with an empty body before that handler runs; a declared length above 8 MiB is still 413 with the same detail string.Residual versus vLLM, unverified on this commit. After the thread tax, the operator's persistent-thread serve was about 48–65 ms against a vLLM p50 of about 32 ms, roughly 1.55×. That remainder is prefill and the joint head, not this PR. The recorded Public231 primary is still tip
594528a9(p50 164.969 ms, p95 216.755 ms) from the old Python HTTP server. Do not cite those numbers as a result foromni-clef.Still not done
torch.compile, and cross-request batching.594528a9.Test Plan
System1-Omni Version / Commit:
a7d3f2b2661099e8fa162aab43aa9a541edf6669oncursor/clef-rust-runtime-734c, basemain05075d0f36e399e3a001ffedea400c666990cda9.From the repository root, after installing pytest:
PYTHONPATH=src python -m pytest tests/clef -q PYTHONPATH=src python -m compileall -q src/models/clef src/frontend/clef.py tests/clef cargo fmt --all --check cargo clippy --workspace --locked --all-targets --features omni-laya/serve -- -D warnings cargo test --workspace --locked --features omni-laya/serveNo checkpoint and no GPU. The Rust serve tests spawn
tests/clef/fake_worker.py, which implements the same frame protocol and reports its pid and thread.Test Result
Run in this environment (no GPU):
PYTHONPATH=src python -m pytest tests/clef -q— 36 passed.compileallon the Clef Python tree — passed.cargo test --locked -p omni-clef— 11 passed, including concurrent posts sharing one executor pid and its main thread, serialization across a delay, a stalled body not holding admission, warmup-before-return, and a dead executor not hanging the next request.cargo clippy --workspace --locked --all-targets --features omni-laya/serve -- -D warnings— passed.cargo test --workspace --locked --features omni-laya/serve— passed.cargo fmt --all --check— passed.Not run:
mkdocs build --strict. Lab L20X load, smoke, answer parity, and Public231 C1. CPU tests do not establish that the joint-head forward got faster.Demo / evidence
N/A for a screen recording: this change is a serve-path and admission change, and the GPU campaign was out of scope.
Unverified: Public231 C1 and a single-request smoke on the L20X-labeled host for commit
a7d3f2b. The 110 ms thread diagnosis and the ~1.55× residual versus vLLM are the operator report cited above, not a result of this commit. Historical p50/p95 figures in the recipe remain the previousThreadingHTTPServerworker.How to run, from the repository root, after the recipe's Python install and snapshot download:
GET /healthis absent until the startup forward returns.Self-review
Before marking this PR ready for review or requesting maintainer review, complete
the self-review checklist.
Keep the PR in draft while this work is incomplete.
For agent assistance, use the optional precheck-pr skill.