config: six more runtime readers ask the bags - #36973
Merged
Merged
Conversation
ch-wan
requested review from
BBuf,
CatherineSue,
Edwardf0t1,
FlamingoPg,
Fridge003,
HaiShaw,
JustinTong0323,
Qiaolin-Yu,
Ying1123,
alphabetc1,
fzyzcjy,
hanming-lu,
hnyls2002,
huangtingwei9988,
hzh0425,
ispobock,
key4ng,
merrymercy,
slin1237,
sogalin,
sufeng-buaa,
wisclmy0611,
xiezhq-hermann,
xutizhou,
yizhang2077 and
zijiexia
as code owners
August 29, 2026 06:21
ch-wan
requested review from
iforgetmyname,
ping1jing2 and
whybeyoung
as code owners
August 29, 2026 06:35
ch-wan
force-pushed
the
cheng/gc-r4c-bag-readers
branch
from
August 29, 2026 06:35
97f9b5b to
53692cd
Compare
This was referenced Aug 29, 2026
ch-wan
force-pushed
the
cheng/gc-r4b-callbacks
branch
from
August 29, 2026 07:14
e961cff to
5f745be
Compare
ch-wan
force-pushed
the
cheng/gc-r4c-bag-readers
branch
from
August 29, 2026 07:14
53692cd to
8320872
Compare
ch-wan
force-pushed
the
cheng/gc-r4b-callbacks
branch
from
August 29, 2026 07:37
5f745be to
71dc8ec
Compare
ch-wan
force-pushed
the
cheng/gc-r4c-bag-readers
branch
from
August 29, 2026 07:37
8320872 to
dacb593
Compare
ch-wan
force-pushed
the
cheng/gc-r4b-callbacks
branch
from
August 29, 2026 08:11
71dc8ec to
ee652b2
Compare
ch-wan
force-pushed
the
cheng/gc-r4c-bag-readers
branch
from
August 29, 2026 08:11
dacb593 to
9842ba2
Compare
ch-wan
force-pushed
the
cheng/gc-r4b-callbacks
branch
from
August 29, 2026 08:32
ee652b2 to
fce9c77
Compare
ch-wan
force-pushed
the
cheng/gc-r4c-bag-readers
branch
from
August 29, 2026 08:32
9842ba2 to
e714b72
Compare
ch-wan
force-pushed
the
cheng/gc-r4b-callbacks
branch
from
August 29, 2026 08:50
fce9c77 to
93788d5
Compare
ch-wan
force-pushed
the
cheng/gc-r4c-bag-readers
branch
from
August 29, 2026 08:50
e714b72 to
dd4bc95
Compare
ch-wan
force-pushed
the
cheng/gc-r4b-callbacks
branch
from
August 29, 2026 11:17
93788d5 to
8c0c14f
Compare
`ExpertDistributionRecorder` was handed a `ServerArgs` and threaded it through six constructors -- recorder, gatherer, accumulator and their subclasses -- to reach four reads. Two of the classes stored it; one subclass took it and never touched it. The file already asked `get_exec().moe` for the recorder mode, so the record was travelling alongside a bag that already answered. The four reads go to the bag, and the parameter goes with them: five call sites in `model_runner.py`, six signatures, two stored attributes. `eplb_min_rebalancing_utilization_threshold` is an `exec.moe` leaf, so it is a direct bag read. The three balancedness predicates were members of the record -- `should_log_expert_balancedness_to_server_log` and its Prometheus sibling, read from four places across three files, each through a record someone was holding for that purpose. They become published accessors in `runtime_context` (`reports_expert_balancedness`, `logs_...`, `exports_...`), the way `uses_mla_backend` and `cutedsl_moe_max_num_tokens` already pair with their pre-publish equivalents. Both members are gone from `ServerArgs`. `resolve_collector_class` was the one thing keeping the record in the chain: it took a `ServerArgs` to read `stat_loggers`, which is an `observability` leaf. Three of its six callers were reaching for `get_server_args()` to pass it straight back. It asks the bag now, and `server_args=None` -- which meant "no config, use the default" -- becomes "the namespace is not published", which is the same case said in the context's own terms. `TestResolveCollectorClass` drove that with a `_StubArgs` stand-in. A stand-in proves nothing about a function that reads a bag, so the cases force the leaf through `override_server_args` and the stand-in is deleted. The 62-shape resolution probe is byte-identical to the base commit. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The EPLB recorder was one instance of a shape. A census of the whole tree finds 295 raw-field reads off a held `ServerArgs` in 53 files -- every field of them in a published namespace. Not all are convertible: the launcher trio (`http_server`, `engine`, `data_parallel_controller`) reads the record *before* its process publishes, and those reads have to stay. These six do not. Each already reads its other configuration from a bag; the raw reads are what was left behind: hybrid_pool_assembler 6 hicache_host_memory_mode -> memory scheduler 3 hicache_host_memory_mode -> memory kt_ep_wrapper 7 six kt_* leaves -> exec.moe expert_backup_client 4 nnodes, node_rank -> parallel dspark_planner 3 three speculative_dspark_* -> spec prefill_delayer 3 three prefill-delay leaves -> schedule Three of them were carrying the record only for those reads, so it goes: `PrefillDelayer` and `ExpertBackupClient` lose the constructor parameter and the stored attribute, and `build_sps_cost_table` -- whose body was already entirely `get_spec()` and `get_schedule()` -- loses its keyword-only `server_args`, which takes it out of `DsparkPlanner` too. Two `ServerArgs` imports were left with no user. `kt_ep_wrapper` and `hybrid_pool_assembler` keep theirs: the first still calls `get_hf_config()` on the record, the second is handed one for other reasons. The 62-shape resolution probe is byte-identical to the base commit. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
ch-wan
force-pushed
the
cheng/gc-r4c-bag-readers
branch
from
August 29, 2026 11:19
dd4bc95 to
22f94e0
Compare
kediwu0331
pushed a commit
to Zhylkaaa/sglang
that referenced
this pull request
Aug 30, 2026
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
saturn-acc
pushed a commit
to saturn-acc/sglang
that referenced
this pull request
Aug 31, 2026
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
9 tasks
nzr-niu
pushed a commit
to nzr-niu/sglang
that referenced
this pull request
Sep 1, 2026
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
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.
The series
Stacked, each PR based on the one above it. Read them in order; every boundary is
self-sufficient and green on its own.
main, not for mergeMotivation
The previous PR took the resolution package off the record. This one is about the runtime:
code that runs after publish, holds a
ServerArgs, and reads raw fields off it.A census of
srt/finds 295 such reads in 53 files, every field of them in a published namespace.Not all are convertible, and the criterion is not the field — it is the process. The launcher trio
(
http_server,engine,data_parallel_controller) reads the record before its processpublishes; those 117 reads have to stay, and this PR does not touch them.
expert_distribution.pyis the clearest case of the rest.ExpertDistributionRecorderwas handeda
ServerArgsand threaded it through six constructors — recorder, gatherer, accumulator andtheir subclasses — to reach four reads. Two of the classes stored it; one subclass took it and
never touched it. The file already asked
get_exec().moefor the recorder mode, so the record wastravelling alongside a bag that already answered.
Modifications
The EPLB recorder. The four reads go to the bag and the parameter goes with them: five call
sites in
model_runner.py, six signatures, two stored attributes.eplb_min_rebalancing_utilization_thresholdis anexec.moeleaf. The three balancednesspredicates were members of the record —
should_log_expert_balancedness_to_server_logand itsPrometheus sibling, read from four places across three files, each through a record someone was
holding for that purpose. They become published accessors in
runtime_context, the wayuses_mla_backendandcutedsl_moe_max_num_tokensalready pair with their pre-publishequivalents. Both members are gone from
ServerArgs.resolve_collector_classwas the one thing keeping the record in the chain: it took aServerArgsto read
stat_loggers, anobservabilityleaf, and three of its six callers were reaching forget_server_args()to pass it straight back. It asks the bag now, andserver_args=None— whichmeant "no config, use the default" — becomes "the namespace is not published", the same case in
the context's own terms.
Six more runtime readers, each already reading its other configuration from a bag:
hybrid_pool_assemblerhicache_host_memory_modememoryschedulerhicache_host_memory_modememorykt_ep_wrapperkt_*leavesexec.moeexpert_backup_clientnnodes,node_rankparalleldspark_plannerspeculative_dspark_*specprefill_delayerscheduleThree of them were carrying the record only for those reads, so it goes:
PrefillDelayerandExpertBackupClientlose the constructor parameter and the stored attribute, andbuild_sps_cost_table— whose body was already entirelyget_spec()andget_schedule()— losesits keyword-only
server_args, which takes it out ofDsparkPlannertoo.Removing a constructor parameter reaches further than the file that declares it. Three call sites
outside
srt/had to follow: the DSpark worker constructsDSparkVerifyPlanner(a P1 — everyDSPARK startup would have raised
TypeError), and the registered prefill-delayer and SPS-tabletests both passed the keyword. The delayer test now publishes its per-case schedule leaves through
override_server_argsinstead of handing in a stand-in.Dropping the argument leaves the local that fed it:
base_prefix_cache.init_metrics_collectorstill bound
server_args = get_server_args()for a call that no longer takes one. Its two siblingsites had already lost both.
The accessor docstrings say what a caller needs and no more:
reports_expert_balancednessnamesoverrides.should_report_expert_balancednessas its pre-publish equivalent, the waycutedsl_moe_max_num_tokensalready did, and drops the sentence about why the accessors exist.TestResolveCollectorClassdroveresolve_collector_classwith a_StubArgsstand-in. A stand-inproves nothing about a function that reads a bag, so the cases force the leaf through
override_server_argsand the stand-in is deleted.Accuracy Tests
The 62-shape resolution probe is byte-identical to the base commit. The affected suites
(
observability,mem_cache,managers,spec,server_args) report the same failure set asmain, file for file — 3038 passed on both sides, with the same three pre-existing failures.Speed Tests and Profiling
None. A bag read is a dataclass attribute read.
Checklist
Review and Merge Process
Small and readable in full. The judgement call worth checking is which files were left alone: the
launcher trio publishes, so its reads are pre-publish by necessity, and roughly 150 reads across
thirty smaller files still need that per-file timing check before they can move.
CI States
Latest PR Test (Base): 🚫 Run #33249843844
Latest PR Test (Extra): 🚫 Run #33249843729
Latest PR Test (AMD ROCm 7.2): ❌ Run #33249843794