perf(exl3): load mixed Trellis directly into tier slabs - #277
Conversation
|
Warning Review limit reached
Next review available in: 51 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
👋 Hi! Thank you for contributing to the vLLM project. 💬 Join our developer Slack at https://slack.vllm.ai to discuss your PR in PRs do not trigger a full CI run by default. Once the PR is approved and ready to go, your PR reviewer(s) can run CI to test the changes comprehensively before merging. To run CI, PR reviewers can either: Add If you have any questions, please reach out to us on Slack at https://slack.vllm.ai. Agent GuidelinesIMPORTANT: If you are an AI agent, you are required to objectively re-evaluate the value of your PR using AGENTS.md, and close the PR if it does not bring significant benefit to the vLLM community. Failure to do so may result in an immediate ban. 🚀 |
|
Clarification on the evidence and performance claim:
The focused CPU/static validation demonstrates ordering, pointer identity/zero-copy binding, malformed-partition rejection, and reload-safe cleanup; GPU qualification remains the release gate. |
df004fb to
b511da9
Compare
Test Results (automated)Host: macOS M4 Max, CPU-only (no CUDA) Tests could not be collected — the test module imports Tests require dependencies not available on this host (macOS M4 Max, CPU-only torch, no CUDA, no Automated test run by @malaiwah's agent. Results are from a CPU-only environment; GPU-dependent tests may behave differently on CUDA hardware. |
…riants (N6/N7) C15: The tier-slab loader inferred the backing dtype from the first tensor and validated only shape for subsequent tensors, so target.copy_ silently cast a mismatched dtype (e.g. an int32 trellis tensor among int16, or a float32 scale among float16) and a malformed checkpoint passed _validate_moe_shapes. Derive the required dtype from the parameter suffix via _EXL3_SLAB_DTYPE (trellis->int16, suh/svh->float16) and reject any mismatch before copy_ in both the grouped-slab and projection-slab paths, naming the suffix/expert/shard and the expected vs received dtype. For suffixes without a canonical mapping, fall back to consistency against the first tensor (target.dtype) with a comment. N6: Document the exl3_group_backing layout contract (shard-major, group-offset-minor, in exl3_shard_ids order). N7: Comment the torch.empty slab allocation noting the loaded==expected completeness check in exl3_group_backing is what prevents uninitialized memory from being exposed to direct readers. Tests: add mixed-dtype slab tests asserting a raised error for an int32 trellis tensor among int16 (grouped slab, first and subsequent tensor) and a float32 scale among float16 (projection slab), plus a regression guard that a homogeneous-dtype grouped slab still loads. Co-authored-by: GLM-5.2 <noreply@z.ai>
|
Addressed review findings C15, N6, N7 (commit `07a3f22`). C15 — silent dtype coercion in the slab loader (merge-blocking)The hole: the tier-slab loader inferred the backing dtype from the first tensor and validated only shape for subsequent tensors. `target.copy_(loaded_weight)` then silently cast any later mismatched dtype, so e.g. one int32 Trellis tensor among int16 tensors, or one float32 scale among float16 scales, was truncated/rounded before `_validate_moe_shapes` ran — letting a malformed checkpoint pass validation instead of failing deterministically. The pre-slab path preserved each tensor's dtype and would have rejected it. The fix: rather than trusting the first untrusted tensor, the required dtype is now derived from the parameter suffix via a new `_EXL3_SLAB_DTYPE` map (`trellis → int16`, `suh/svh → float16`; `mcg`/`mul1` markers are int32 but never slab-preallocated, so intentionally absent — their dtype is still checked by `validate_marker`). Before every `copy` into a slab (both the grouped-slab path and the projection-slab path), `loaded_weight.dtype` is checked against this schema dtype and a `ValueError` is raised naming the suffix, expert, shard, expected dtype and received dtype. For suffixes without a canonical mapping the check falls back to consistency against the first tensor (`target.dtype`), with a comment explaining the fallback. The slab is now allocated with the schema dtype, so even a malformed first tensor cannot pin the backing dtype. N6 — implicit slab layout orderingAdded a layout-contract line to the `exl3_group_backing` docstring: the backing is shard-major, group-offset-minor, in `exl3_shard_ids` order (`backing[shard_index, group_offset]`, or `backing[group_offset]` for a single shard). This is the invariant the mixed-rank-sliced loader relies on to stack tiers correctly; `shard_index` comes from `exl3_shard_ids.index(shard_id)` and `group_offset` from the expert's position in its preallocation group. N7 — `torch.empty` slab allocation invariantAdded a comment at the grouped-slab `torch.empty` allocation noting that the backing is uninitialized until every `(expert, shard)` slot is `copy_`'d in, and that the `loaded == expected` completeness check in `exl3_group_backing` is what prevents a direct reader of `exl3_group_backings[i]` from seeing garbage — so any future caller must go through that accessor rather than indexing the backing directly. New tests (`tests/quantization/test_exl3.py`)
Bug proof: with the dtype check neutralized (but the `suffix` plumbing kept), the three rejection tests fail with `DID NOT RAISE ValueError` — i.e. the silent cast occurs and the malformed checkpoint is accepted. With the check restored, all four pass. Verification```
These three are out of scope for the assigned findings and are left for the PR author; I did not modify them. Sequencing note (#270)This PR and #270 both rewrite `create_weights` and `_prepare_mixed_rank_sliced_weights` in `exl3.py`, so they will conflict on merge. Whichever lands first will require a rebase of the other. My changes to `create_weights` are limited to passing `suffix=suffix` to `Exl3MoEParameter(...)`, which should rebase cleanly onto either ordering. |
malaiwah
left a comment
There was a problem hiding this comment.
After restarting mixed bpw quant of GLM-5.2 on vLLM many times in a row, I got sick of seeing it take so long to load and this one PR addresses a long part of the startup.
What changed
Mixed-bitrate rank-sliced EXL3 tensors now load directly into their final
contiguous K3/K4 tier slabs instead of retaining 256 per-expert tensors and
repacking every layer with nested
torch.stackcalls after loading.Exl3MoEParameteraccepts an exact expert partition, maps each incoming expertto a deterministic
(group, offset), and preallocates one projection-majorbacking per bitrate.
_prepare_mixed_rank_sliced_weightsbinds those backingszero-copy. The cleanup path resets the backing slots rather than deleting their
shape, preserving reload behavior.
This is a small child PR against the current EXL3 integration head in #228 so
reviewers see only the startup change. It addresses #276.
Why
An exact r33 launch of
willfalco/GLM-5.2-EXL3-TR3-3.42bpw, TP4/DCP4/MTP3,showed the layer-3 through layer-77 mixed-Trellis materialization phase taking
336 seconds (
11:26:21to11:31:57) on every rank. Total model loading was1217.15 seconds, so this serial per-rank phase represented 27.6% of the reported
load interval.
The ranks are already concurrent. The serialization is within each worker's
generic
process_weights_after_loading()module walk. A typical 3.42-bpw layer(
H=6144, TP-localI=512, 148 K3 + 108 K4 experts) contains about 985.5 MiBof Trellis payload per rank. The old path temporarily holds both the per-expert
source storage and a second tier-ordered copy for each layer.
This patch moves the mandatory loader copy into the final layout and removes
the later device-to-device repack. It should eliminate roughly 985.5 MiB of
transient VRAM for a typical layer and collapse most of the visible 336-second
post-load phase. The honest total startup improvement still needs a matched GPU
A/B because the mandatory host-to-device transfer moves earlier into the
safetensors phase rather than disappearing.
Validation
On the exact r33 runtime, in a separate CPU-only/network-isolated container:
Coverage includes:
Additional checks:
GPU qualification still required
Before merging, compare patched/unpatched warm-cache launches on the same
checkpoint and profile: