config: the forwarding slots go; the dispatcher calls the family directly by ch-wan · Pull Request #36792 · sgl-project/sglang

ch-wan · 2026-08-28T05:15:32Z

Motivation

After PR A, the method left behind for each moved handler is three lines of forwarding:

def _handle_cuda_graph_config(self):
    from sglang.srt.arg_groups.cuda_graph_hook import handle_cuda_graph_config

    handle_cuda_graph_config(self)

Ninety-three of those, 403 lines, and every one of them is also why arg_groups/ reaches back
into the record: a moved handler calling a sibling had to go server_args._disable_x() to land
on the slot.

Modifications

The slots go. The dispatcher, the other handlers and the tests call the hook function directly.

Two kinds stay. check_server_args is named by entrypoints/engine.py, so it is a stage the
launch path calls rather than a forwarding artefact — deleting it turned 34 launch shapes into
AttributeError, which is how it was found. And the five that forward into
arg_groups.overrides (_declare, _late_resolution, …) are the record's own declaration
API.

The argument mapping is read off each slot, not assumed. A slot is not always f(self, …):
_validate_mamba_no_buffer(self, view, arch) forwards validate_mamba_no_buffer(view, arch)
and drops the receiver, _validate_mamba_extra_buffer forwards one attribute of it rather than
the record, and a test may call ServerArgs._x(args) unbound, where the instance is argument
zero. Guessing any of these produces a call with the wrong arity.

The slots were doing one thing besides forwarding, and it has to survive them. Their import
was function-local, and that is what kept server_args importable: a hook module reaches
server_args again through its own imports (validation_hook → kv_events → server_args),
so hoisting them to module scope cycles. The record now imports each hook function at the top
of the function that calls it.

ServerArgs goes from 6028 to 5609 lines, and arg_groups/ from 127 callbacks into the record
to 96 — what is left there is the read surface (get_model_config and its kin) and the
declaration API, with no pure forwarding among them.

Four test seams move with the code. A patch.object(ServerArgs, "_disable_…") becomes a patch
of the hook module. Two guards learn the dispatcher's second call spelling — a step is now
either self._x() or a bare-name call into arg_groups/. Two completeness floors in
test_resolution_reads_the_declarations were calibrated to the old class: what the dispatcher
still reaches inside it is the five read wrappers, pinned by name rather than counted, and the
package side is covered by test_no_hook_reads_a_field_off_the_record.

The fourth is the one to watch, because it is the seam a mechanical rewrite gets wrong in a way
that passes here and fails in CI. TestServerArgsIBDeviceValidation has a helper named
_validate_ib_devices that builds a dummy record and mocks the sysfs listing; the record had a
slot of the same name. Rewriting the call site by name turned self._validate_ib_devices(x)
into a direct call with the TestCase as server_args, which skips the mock — invisible on a
box that has /sys/class/infiniband, a RuntimeError on the CPU runner this file is
registered for. The three tests call the helper again.

The second commit is the one to read for cost. Removing a slot removes its function-local
import, and that import was load-bearing twice: at module scope in server_args.py it cycles,
and at the top of the calling function it is still too early —
_run_resolution_pipeline returns for a dummy model after five steps, so importing all
eighteen families up front took ServerArgs(model_path="dummy").resolve_once() from 0.014s to
1.82s, on the path every override_server_args in the test suite goes through. Each import now
sits immediately before the first call it serves, and a hook module that calls another imports
it inside the calling function rather than at module scope. A dummy resolution is back to
0.016s and six hook modules. TestResolutionStaysLazy pins both halves.

Accuracy Tests

62 launch shapes report the same exception type and message as the base.
The 24-shape × 478-field resolution dump is identical.
The 159 registered tests that mention the record, tip vs base: no regression in either
direction.

Speed Tests and Profiling

None. One frame of forwarding is removed from each handler call; the import is lazy either way.

Checklist

Format your code according to the Format code with pre-commit.
Add unit tests according to the Run and add unit tests.
Update documentation according to Write documentations.
Provide accuracy and speed benchmark results according to Test the accuracy and Benchmark the speed.
Follow the SGLang code style guidance.

Review and Merge Process

Most of the diff is one deletion repeated 93 times and its call sites. The parts worth reading
are the import placement in server_args.py (function-local, deliberately) and the three test
seams at the end.

Ping Merge Oncalls to start the process. See the PR Merge Process.
Get approvals from CODEOWNERS and other reviewers.
Trigger CI tests with comments or contact authorized users to do so.
After green CI and required approvals, ask Merge Oncalls or people with Write permission to merge the PR.

CI States

Latest PR Test (Base): ❌ Run #33194688106
Latest PR Test (Extra): 🚫 Run #33194684875
Latest PR Test (AMD ROCm 7.2): 🚫 Run #33194684743

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 4838e1c400

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector · 2026-08-28T06:06:38Z

    def test_validate_ib_devices_accepts_comma_separated(self):
        self.assertEqual(
-            self._validate_ib_devices("mlx5_0, mlx5_1"),
+            validate_ib_devices(self, "mlx5_0, mlx5_1"),


Route IB validation tests through the patched helper

On CPU CI runners without /sys/class/infiniband devices named mlx5_0 through mlx5_3, this direct call bypasses _validate_ib_devices, which supplies the mocked sysfs contents, so validate_ib_devices raises RuntimeError or rejects the expected devices. The JSON-object and JSON-file tests were changed the same way; call self._validate_ib_devices(...) in all three tests so this registered CPU suite remains hardware-independent.

Useful? React with 👍 / 👎.

…ctly Once a handler's body moved to `arg_groups/`, the method left behind was three lines of forwarding. Ninety-three of them are gone: the dispatcher, the other handlers and the tests call the hook function directly. Two kinds of slot stay. `check_server_args` is named by `entrypoints/engine.py`, so it is a stage the launch path calls rather than a forwarding artefact; and the five that forward into `arg_groups.overrides` (`_declare`, `_late_resolution`, ...) are the record's own declaration API. The argument mapping is read off each slot rather than assumed. A slot is not always `f(self, ...)`: `_validate_mamba_no_buffer(self, view, arch)` forwards `validate_mamba_no_buffer(view, arch)` and drops the receiver, and `_validate_mamba_extra_buffer` forwards one attribute of it rather than the record. Guessing either way produces a call that takes the wrong number of arguments. The slots were doing something besides forwarding, and it has to survive them: their import was **function-local**, which is what kept `server_args` importable at all. A hook module reaches `server_args` again through its own imports (`validation_hook` -> `kv_events` -> `server_args`), so hoisting those imports to module scope cycles. The record now imports each hook function at the top of the function that calls it. The record loses 403 lines and `arg_groups/` loses 31 of its 127 callbacks into the record; what is left there is the read surface (`get_model_config` and its kin) and the declaration API, with no pure forwarding among them. Three test seams move with the code: a `patch.object(ServerArgs, "_disable_...")` becomes a patch of the hook module, and two guards learn the dispatcher's second call spelling. Two completeness floors in `test_resolution_reads_the_declarations` were calibrated to the old class -- what the dispatcher still reaches inside the class is a handful of read wrappers now, and the package side is covered by `test_no_hook_reads_a_field_off_the_record`. 62 launch shapes and the 24-shape resolution dump are identical, and the 159 registered tests that mention the record report the same failure set as the base. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Removing the forwarding slots removed their function-local import with them, and that import was load-bearing twice over. Hoisting them to module scope in `server_args.py` cycles (a hook reaches `server_args` again through its own imports), which the first version of this commit already worked around by importing at the top of each calling function. That is still too early: `_run_resolution_pipeline` returns for a dummy model after five steps, so importing all eighteen families at the top made `ServerArgs(model_path="dummy").resolve_once()` take 1.82s instead of 0.014s -- on the path every `override_server_args` in the test suite goes through. Each import now sits immediately before the first call it serves, and the same applies inside `arg_groups/`: a hook module that calls another imports it in the function that calls, not at module scope, so loading one family does not drag in a family it may never reach. A dummy resolution is back to 0.016s and 19 new modules, matching the parent commit, and loads six hook modules rather than eighteen. `TestResolutionStaysLazy` pins both halves: no module-level hook-to-hook import, and a dummy resolution that does not reach the model, cuda-graph or attention families. Reported by the codex reviewer on the slot cut, with the timing measured. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Twenty-four members of `ServerArgs` were still called back into from `arg_groups/` -- 96 call sites, 503 lines. Fourteen of them have no other caller at all: not elsewhere in `srt`, not in a test. They are resolution-time helpers that ended up on the record for historical reasons, and they move to the family that calls them. `generate_{decode,prefill,cpu}_*_batch_sizes` and `apply_cuda_graph_disaggregation_roles` join `cuda_graph_hook`; `reserve_for_graph_mb`, `reserve_for_deepep_a2a_mb` and `adjust_mem_fraction_for_vlm` join `memory_hook`; the two dispatch-token budgets join `moe_hook`; `is_mistral_native_format` joins `model_path_hook`; `is_attention_backend_not_set` and `get_default_attn_backend` join `overrides`. The two `_set_default_dsa_*` members were one-line forwards to a post-process pass, like the ninety-three slots #36792 cut, and are now that call. `ServerArgs` goes from 5277 to 4896 lines. `is_attention_backend_not_set` takes the view rather than the record. Every read in it is a view read, three of its call sites hold a view rather than a record, and nine of the other ten already had one in scope. `get_default_attn_backend` needs both overlays, so it keeps the record; the one call site that has only a view reaches the record through `record_of`. Two things this makes visible: - `get_default_attn_backend` read `tp_size` off the record -- raw input, not what resolution decided. `test_no_hook_reads_a_field_off_the_record` walks `arg_groups/` and could not see it while it was a method on the class. It reads through the view now. - `hisparse_hook._is_hip` imported `is_hip` *from `server_args`*, so that the tests would have a stable patch target. That made a configuration module the home of a platform probe. It asks `utils.common` now, and the eight patches name `hisparse_hook._is_hip`. Twelve `is_attention_backend_not_set` accessor stubs on fake records are retired. Four of them recomputed the predicate from the fake's own fields -- exactly what the function now does -- and the rest are replaced by the field values that make it true or false. A stubbed accessor stops intercepting the moment its member moves, so it was hiding what these fixtures actually said. The 62-shape resolution probe is byte-identical to the base commit. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

…ctly (sgl-project#36792) Co-authored-by: Claude Opus 5 <noreply@anthropic.com>

github-actions Bot added lora Multi-modal multi-modal language model hicache Hierarchical Caching for SGLang npu labels Aug 28, 2026

ch-wan mentioned this pull request Aug 28, 2026

[CI vehicle, do not merge] config: round-3 ServerArgs series (#36789-#36792) #36795

Closed

5 tasks

ch-wan force-pushed the cheng/gc-r3c-readers branch from aafdb2c to a17269c Compare August 28, 2026 06:02

ch-wan requested review from BBuf, Edwardf0t1, Fridge003, HaiShaw, Ying1123, alphabetc1, hanming-lu, hnyls2002, huangtingwei9988, hzh0425, ispobock, merrymercy, xiezhq-hermann and yizhang2077 as code owners August 28, 2026 06:02

ch-wan force-pushed the cheng/gc-r3d-cut-slots branch from 123e4ce to 4838e1c Compare August 28, 2026 06:02

chatgpt-codex-connector Bot reviewed Aug 28, 2026

View reviewed changes

ch-wan force-pushed the cheng/gc-r3c-readers branch from a17269c to 02f34bf Compare August 28, 2026 06:11

ch-wan force-pushed the cheng/gc-r3d-cut-slots branch from 4838e1c to bc415e8 Compare August 28, 2026 06:11

ch-wan force-pushed the cheng/gc-r3c-readers branch from 02f34bf to 1a7b098 Compare August 28, 2026 06:51

ch-wan force-pushed the cheng/gc-r3d-cut-slots branch from bc415e8 to 43ac66b Compare August 28, 2026 06:51

ch-wan force-pushed the cheng/gc-r3c-readers branch from 1a7b098 to 2b1506c Compare August 28, 2026 07:09

ch-wan force-pushed the cheng/gc-r3d-cut-slots branch from 43ac66b to 7063b1a Compare August 28, 2026 07:09

ch-wan force-pushed the cheng/gc-r3c-readers branch from 2b1506c to 3bddc80 Compare August 28, 2026 07:15

ch-wan closed this Aug 28, 2026

ch-wan force-pushed the cheng/gc-r3d-cut-slots branch from 7063b1a to 3bddc80 Compare August 28, 2026 07:15

ch-wan reopened this Aug 28, 2026

ch-wan force-pushed the cheng/gc-r3c-readers branch from 3bddc80 to 91f86bf Compare August 28, 2026 07:52

ch-wan force-pushed the cheng/gc-r3d-cut-slots branch from 0c61367 to 35f3311 Compare August 28, 2026 07:52

ch-wan force-pushed the cheng/gc-r3c-readers branch from 91f86bf to 2d675da Compare August 28, 2026 17:13

ch-wan force-pushed the cheng/gc-r3d-cut-slots branch from 35f3311 to fa1d65c Compare August 28, 2026 17:13

ch-wan force-pushed the cheng/gc-r3c-readers branch from 2d675da to e37db74 Compare August 28, 2026 17:21

Base automatically changed from cheng/gc-r3c-readers to main August 28, 2026 17:21

ch-wan and others added 2 commits August 28, 2026 17:21

ch-wan force-pushed the cheng/gc-r3d-cut-slots branch from fa1d65c to 1690874 Compare August 28, 2026 17:26

ch-wan merged commit ef20fab into main Aug 28, 2026
3 of 7 checks passed

ch-wan deleted the cheng/gc-r3d-cut-slots branch August 28, 2026 17:26

ch-wan mentioned this pull request Aug 28, 2026

config: the resolution pipeline's dispatcher leaves the record #36896

Merged

5 tasks

ch-wan mentioned this pull request Aug 29, 2026

config: the resolution callbacks into the record go to zero #36972

Merged

5 tasks

saturn-acc pushed a commit to saturn-acc/sglang that referenced this pull request Aug 31, 2026

config: the forwarding slots go; the dispatcher calls the family dire…

0a858b1

…ctly (sgl-project#36792) Co-authored-by: Claude Opus 5 <noreply@anthropic.com>

nzr-niu pushed a commit to nzr-niu/sglang that referenced this pull request Sep 1, 2026

config: the forwarding slots go; the dispatcher calls the family dire…

288f99b

…ctly (sgl-project#36792) Co-authored-by: Claude Opus 5 <noreply@anthropic.com>

Navigation Menu

Uh oh!

config: the forwarding slots go; the dispatcher calls the family directly - #36792

config: the forwarding slots go; the dispatcher calls the family directly#36792
ch-wan merged 2 commits into
mainfrom
cheng/gc-r3d-cut-slots

ch-wan commented Aug 28, 2026 •

edited by github-actions Bot

Loading

Uh oh!

chatgpt-codex-connector Bot left a comment

Uh oh!

chatgpt-codex-connector Bot Aug 28, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

ch-wan commented Aug 28, 2026 • edited by github-actions Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Motivation

Modifications

Accuracy Tests

Speed Tests and Profiling

Checklist

Review and Merge Process

CI States

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot Aug 28, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

ch-wan commented Aug 28, 2026 •

edited by github-actions Bot

Loading