perf(r3): accelerate routed-expert transport with packed arrays - #1909
Merged
erictang000 merged 3 commits intoAug 3, 2026
Conversation
This was referenced Jul 16, 2026
Contributor
|
Warning Gemini encountered an error creating the review. You can try again by commenting |
dyurk-lila
force-pushed
the
upstream/r3-packed-transport
branch
from
July 20, 2026 20:25
fb01ce9 to
fb86814
Compare
dyurk-lila
force-pushed
the
upstream/r3-packed-transport
branch
from
July 28, 2026 23:28
fb86814 to
b390bc8
Compare
Serializing routed-expert IDs as nested JSON lists creates a large HTTP payload and substantial Python/JSON overhead before training sees the data. - Compact routed-expert IDs to the smallest safe uint8, int16, or int32 dtype. - Base64-encode the contiguous buffer alongside its shape and dtype, and decode it back into a validated compact NumPy array on the client. - Serialize the vLLM response and parse client responses with orjson. - Gate packed routes behind the existing routed-expert request flag. - Accept the decoded NumPy route arrays at preprocessing, validating dtype and shape before padding and tensor conversion. Because orjson emits JSON null rather than raising for non-finite floats, the sampled-token logprob path floors missing and non-finite values to -9999.0 -- the same floor vLLM applies at its own serving boundaries. This keeps one bad logprob from failing the whole generate request. Note the isfinite test also catches NaN, which vLLM's max(logprob, -9999.0) misses since max returns its first argument on a False comparison. This change is intentionally scoped to transport and preprocessing: it does not alter batching, Experience, device placement, or backend replay setup.
dyurk-lila
force-pushed
the
upstream/r3-packed-transport
branch
from
July 29, 2026 21:07
b390bc8 to
a81e61d
Compare
Collaborator
|
/gemini review |
Contributor
There was a problem hiding this comment.
Code Review
This pull request optimizes the serialization and transmission of routed expert indices and logprobs between the vLLM server and the remote inference client by introducing a binary payload contract using base64-encoded NumPy arrays. Key feedback includes safely detaching and moving CUDA tensors to the CPU before NumPy conversion in the server actor, using more robust dictionary retrieval for logprobs to prevent potential attribute errors, and relaxing shape dimension type checks to accommodate NumPy integer types.
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.
Problem
The routed-expert (R3) generation path returns one expert ID for every generated token, MoE layer, and top-k slot. Serializing that integer tensor as nested JSON lists creates a large HTTP payload and substantial Python/JSON overhead before training sees the data.
Implementation
uint8,int16, orint32NumPy dtype.orjson.This change is intentionally scoped to transport and preprocessing: it does not alter batching,
Experience, device placement, or backend replay setup.Also included: non-finite sampled logprobs
The
/skyrl/v1/generateendpoint previously raised on any non-finite logprob, on theassumption that
orjsonwould refuse to serialize it.orjsonactually emits JSONnull, so the guard was unnecessary — and vLLM does report a non-finite logprob for atoken it just sampled (roughly one per 42 minutes of rollouts at 192-GPU scale).
FastAPI maps the
ValueErrorto HTTP 400, which callers treat as a fatal invariantviolation, so a single token could tear down an entire run.
Non-finite values are now floored to
-9999.0, matching what vLLM itself does at itsserving boundaries, with the pre-existing missing-entry branch folded into the same
path. Note
isfinitealso catchesNaN, which vLLM's ownmax()floor lets throughbecause
maxreturns its first argument on aFalsecomparison. The loop is extractedinto a vLLM-free helper (
logprobs_wire.py) so it is unit-testable without a liveengine, and logs a warning identifying the request and clamp count.
Testing
CPU tests run with:
The suite covers route preprocessing, dtype selection, base64 round trips, malformed payloads, the packed wire codec, and clamping of missing/
-inf/NaNsampled logprobs.ruffandblack(line length 120) pass on the changed files.This is part of a small series of routed-expert transport improvements.