Repository navigation
perf(engine): avoid timed waits for Engine responses - #39486
Merged
ishandhanani merged 2 commits intoSep 22, 2026
Merged
ishandhanani merged 2 commits into
ishandhanani merged 2 commits into
Conversation
jthomson04
requested review from
Ying1123,
hnyls2002,
merrymercy and
xiezhq-hermann
as code owners
September 14, 2026 23:01
Collaborator
|
/tag-and-rerun-ci |
jthomson04
force-pushed
the
jthomson04/perf-sglang-direct-engine-wait
branch
from
September 15, 2026 16:18
59da1be to
0a5d2c4
Compare
Contributor
Author
|
/rerun-failed-ci |
jthomson04
added a commit
to jthomson04/sglang
that referenced
this pull request
Sep 16, 2026
jthomson04
force-pushed
the
jthomson04/perf-sglang-direct-engine-wait
branch
from
September 17, 2026 18:08
ed1e0f1 to
e814ee4
Compare
jthomson04
added a commit
to jthomson04/sglang
that referenced
this pull request
Sep 17, 2026
Signed-off-by: jthomson04 <jwillthomson19@gmail.com>
Contributor
Author
|
/rerun-failed-ci |
1 similar comment
Contributor
Author
|
/rerun-failed-ci |
Signed-off-by: jthomson04 <jwillthomson19@gmail.com>
Signed-off-by: jthomson04 <jwillthomson19@gmail.com>
jthomson04
force-pushed
the
jthomson04/perf-sglang-direct-engine-wait
branch
from
September 18, 2026 16:16
e814ee4 to
01ff254
Compare
Collaborator
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.
Summary
Engine calls pass no HTTP request object, but each response wait still uses
asyncio.wait_for. Its timeout path checks for an HTTP disconnect and otherwise starts another wait.Await
state.event.wait()directly whenrequest is None. HTTP callers keep the timed wait and disconnect check. Both paths continue through the existing iterator, output handling, and request cleanup. This adds no public API and does not change notification settings or scheduler emission.Validation
pytest -q test/registered/unit/managers/test_tokenizer_manager_rid_cleanup.py.pre-commit run --files python/sglang/srt/managers/tokenizer_manager.py test/registered/unit/managers/test_tokenizer_manager_rid_cleanup.py.The patch changes one conditional in the response iterator and adds coverage to the existing CPU CI test file. It does not change model or kernel code.
Speed Tests and Profiling
In one matched Dynamo SGLang TCP push comparison, this wait change improved output token throughput by 37.7% at
batch_notify_size=16, SGLang's default. Both controls used fixed concurrency 1024, ISL/OSL 33/1024,stream_interval=1, and no worker runtime thread override. Only the response wait changed.Throughput uses completed output tokens divided by actual duration including drain. Each control used 2048 separate warmup requests, a 120-second measurement, and a 90-second grace period. Both controls completed without request errors and had exact token lengths. Separate diagnostic captures retained one token per token-bearing backend frame. TTFT tails increased, as shown above.
For context, native SGLang HTTP serving in the same campaign had 45.243 seconds p99 TTFT, versus 1.375 seconds for the patched Dynamo control. Both used notification size 16 and fixed concurrency 1024. Native used 4 tokenizer / 4 detokenizer processes; Dynamo used 1/1. The patched Dynamo TTFT therefore remained well below this native reference, despite increasing relative to the unchanged Dynamo control.
This is one control per configuration, so the result does not establish repeatability or a GIL mechanism. GPU measurements used SGLang main
5c2de3f35567ffceec6cea86ba18e075692e9101plus this patch, before the PR was applied to the current main base.CI States
Latest PR Test (Base): ✅ Run #35367509826
Latest PR Test (Extra): ❌ Run #35367509417
Latest PR Test (AMD ROCm 10): ❌ Run #35367509900