Skip to content

config: retire the last process-global config field reads - #33338

Merged
ch-wan merged 1 commit into
mainfrom
cheng/gc-global-read-sweep
Aug 3, 2026
Merged

ch-wan merged 1 commit into
mainfrom
cheng/gc-global-read-sweep

Conversation

@ch-wan

@ch-wan ch-wan commented Aug 3, 2026

Copy link
Copy Markdown
Collaborator

Part 5 of five, all based on main and meant to merge in order.
Applies on top of parts 1–4. Until they land, this PR's diff includes them.

Replaces #33244, which was closed unmerged: GitHub treated the previous
chained-base series as a stack, which blocks both base retargeting and
every merge path except the async endpoint. The review discussion and the
triage of each round of comments is on #33244; the code here is identical to
that PR's final revision.

What

get_server_args().<field> reads the startup record of one process. Twenty-six of
those reads were left in srt; this removes every one that has somewhere better to
go, and pins the rest.

Nine reads move to the namespace accessors — the value they want is the resolved
one, including post-publish overrides: attention_backend ×5 →
get_exec().kernel, skip_tokenizer_init ×2 → get_serving(), the draft-aware
load_formatget_model(), and a PP size → get_parallel() (live topology, which
is what the DSA indexer wants).

Two reads move to the instance instead of a namespace. The multimodal processor
picked its fast-image-processor device from the process-global config, but
base_gpu_id differs per worker — the encode-server DP workers each specialise their
own copy — so no process-global value can stand in for it, and several Engines can
share a tokenizer process. The decision moves into
BaseMultimodalProcessor._fast_image_processor_device, reading the instance the
processor was constructed with, like the ten other config reads in that class. Branch
order, the NPU preprocess patches and the case that leaves device unset are
unchanged.

What stays is the derived API: mamba_cache_chunk_size (@property),
get_model_config(), enable_mamba_extra_buffer{,_lazy}() — computed from several
fields plus the HF config, so they are not namespace leaves and ServerArgs is their
only home. Two config-intent reads of live-shadowed sizes also stay: get_parallel()
shadows tp/pp/dcp/attn_cp/moe_dp_size with the live topology, and both sites run
where no process group exists (cuda_ipc_transport_utils even guards for "not
published yet").

A new AST ratchet pins field reads at zero and exempts the above by name, with the
reasoning in its docstring.

Test changes

test_dllm_fdfo_kv_reuse rebound allocation.get_server_args to a
SimpleNamespace. That kind of stand-in stops intercepting the moment a reader
migrates — which is exactly what happened here — so it publishes a real config now.
New test_processor_device_selection.py pins the per-instance device resolution
against a conflicting published config.

Validation

Per-area unit suites (mem_cache, multimodal, batch_overlap,
layers/attention, managers) and all ratchets pass. Full registered CPU battery
(16 partitions) against main: seven branch-only failures, each re-run serially and
green — two were port collisions from the parallel run itself, the rest are the
box's standing environment failures. No new failures.

Not covered locally: the flipped reads sit in model and attention paths that need a
GPU to execute; the flips are value-preserving (the bags are projected from the same
published config), but the speculative and model CI suites are the real check.


CI States

Latest PR Test (Base): 🚫 Run #30784352926
Latest PR Test (Extra): 🚫 Run #30784352743

@gemini-code-assist

Copy link
Copy Markdown
Contributor

Caution

The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased.

@ch-wan
ch-wan force-pushed the cheng/gc-global-read-sweep branch 3 times, most recently from c365544 to a4c0885 Compare August 3, 2026 04:24
`get_server_args().<field>` reads one process's startup record. Nine sites still
did that for a value that has a namespace: the attention backend (5),
`skip_tokenizer_init` (2), the draft-aware `load_format`, and a chunked-prefill
size in `sglang.kernels`. They now read `get_exec().kernel` / `get_serving()` /
`get_model()` / `get_schedule()`, so they see the resolved value including
post-publish overrides.

The multimodal processor's device selection moves to the instance it was
constructed with rather than to a namespace: `base_gpu_id` differs per worker (the
encode-server DP workers each specialise their own copy), so no process-global
value can stand in for it, and engines sharing a tokenizer process each need their
own. Branch order, the NPU preprocess patches, and the case that leaves "device"
unset are unchanged.

What stays on `get_server_args()` is the derived API — `@property` and method
members computed from several fields plus the HF config
(`mamba_cache_chunk_size`, `get_model_config()`, `enable_mamba_extra_buffer*`) —
plus three config-intent reads of live-shadowed sizes, each of which needs an
answer the live topology property cannot give (the DSA indexer's PP gate must
short-circuit before touching the PP group, `allocation`'s DCP gate asks whether
DCP was configured at all, and the CUDA-IPC recycler runs where no group exists).

A new AST ratchet pins both shapes it can see — the direct call and an alias
bound from it in the same function — at 0 and 12 respectively, exempting the
derived APIs and those three sites by name. The alias-form baseline is not zero:
those reads are mostly per-runner fields in model code, and lowering them is the
next slice.

Two fixtures stopped faking config: `test_dllm_fdfo_kv_reuse` rebound
`allocation.get_server_args` to a SimpleNamespace, which silently stops
intercepting the moment a reader migrates; it publishes a real config instead.
@ch-wan
ch-wan force-pushed the cheng/gc-global-read-sweep branch from a4c0885 to 38acc74 Compare August 3, 2026 04:24
@ch-wan
ch-wan merged commit b8109b5 into main Aug 3, 2026
6 of 13 checks passed
@ch-wan
ch-wan deleted the cheng/gc-global-read-sweep branch August 3, 2026 04:24
kfhfar pushed a commit to kfhfar/sglang that referenced this pull request Aug 7, 2026
…t#33338)

`get_server_args().<field>` reads one process's startup record. Nine sites still
did that for a value that has a namespace: the attention backend (5),
`skip_tokenizer_init` (2), the draft-aware `load_format`, and a chunked-prefill
size in `sglang.kernels`. They now read `get_exec().kernel` / `get_serving()` /
`get_model()` / `get_schedule()`, so they see the resolved value including
post-publish overrides.

The multimodal processor's device selection moves to the instance it was
constructed with rather than to a namespace: `base_gpu_id` differs per worker (the
encode-server DP workers each specialise their own copy), so no process-global
value can stand in for it, and engines sharing a tokenizer process each need their
own. Branch order, the NPU preprocess patches, and the case that leaves "device"
unset are unchanged.

What stays on `get_server_args()` is the derived API — `@property` and method
members computed from several fields plus the HF config
(`mamba_cache_chunk_size`, `get_model_config()`, `enable_mamba_extra_buffer*`) —
plus three config-intent reads of live-shadowed sizes, each of which needs an
answer the live topology property cannot give (the DSA indexer's PP gate must
short-circuit before touching the PP group, `allocation`'s DCP gate asks whether
DCP was configured at all, and the CUDA-IPC recycler runs where no group exists).

A new AST ratchet pins both shapes it can see — the direct call and an alias
bound from it in the same function — at 0 and 12 respectively, exempting the
derived APIs and those three sites by name. The alias-form baseline is not zero:
those reads are mostly per-runner fields in model code, and lowering them is the
next slice.

Two fixtures stopped faking config: `test_dllm_fdfo_kv_reuse` rebound
`allocation.get_server_args` to a SimpleNamespace, which silently stops
intercepting the moment a reader migrates; it publishes a real config instead.
saturn-acc pushed a commit to saturn-acc/sglang that referenced this pull request Aug 16, 2026
…t#33338)

`get_server_args().<field>` reads one process's startup record. Nine sites still
did that for a value that has a namespace: the attention backend (5),
`skip_tokenizer_init` (2), the draft-aware `load_format`, and a chunked-prefill
size in `sglang.kernels`. They now read `get_exec().kernel` / `get_serving()` /
`get_model()` / `get_schedule()`, so they see the resolved value including
post-publish overrides.

The multimodal processor's device selection moves to the instance it was
constructed with rather than to a namespace: `base_gpu_id` differs per worker (the
encode-server DP workers each specialise their own copy), so no process-global
value can stand in for it, and engines sharing a tokenizer process each need their
own. Branch order, the NPU preprocess patches, and the case that leaves "device"
unset are unchanged.

What stays on `get_server_args()` is the derived API — `@property` and method
members computed from several fields plus the HF config
(`mamba_cache_chunk_size`, `get_model_config()`, `enable_mamba_extra_buffer*`) —
plus three config-intent reads of live-shadowed sizes, each of which needs an
answer the live topology property cannot give (the DSA indexer's PP gate must
short-circuit before touching the PP group, `allocation`'s DCP gate asks whether
DCP was configured at all, and the CUDA-IPC recycler runs where no group exists).

A new AST ratchet pins both shapes it can see — the direct call and an alias
bound from it in the same function — at 0 and 12 respectively, exempting the
derived APIs and those three sites by name. The alias-form baseline is not zero:
those reads are mostly per-runner fields in model code, and lowering them is the
next slice.

Two fixtures stopped faking config: `test_dllm_fdfo_kv_reuse` rebound
`allocation.get_server_args` to a SimpleNamespace, which silently stops
intercepting the moment a reader migrates; it publishes a real config instead.
jakki-amd pushed a commit to jakki-amd/sglang that referenced this pull request Sep 9, 2026
…t#33338)

`get_server_args().<field>` reads one process's startup record. Nine sites still
did that for a value that has a namespace: the attention backend (5),
`skip_tokenizer_init` (2), the draft-aware `load_format`, and a chunked-prefill
size in `sglang.kernels`. They now read `get_exec().kernel` / `get_serving()` /
`get_model()` / `get_schedule()`, so they see the resolved value including
post-publish overrides.

The multimodal processor's device selection moves to the instance it was
constructed with rather than to a namespace: `base_gpu_id` differs per worker (the
encode-server DP workers each specialise their own copy), so no process-global
value can stand in for it, and engines sharing a tokenizer process each need their
own. Branch order, the NPU preprocess patches, and the case that leaves "device"
unset are unchanged.

What stays on `get_server_args()` is the derived API — `@property` and method
members computed from several fields plus the HF config
(`mamba_cache_chunk_size`, `get_model_config()`, `enable_mamba_extra_buffer*`) —
plus three config-intent reads of live-shadowed sizes, each of which needs an
answer the live topology property cannot give (the DSA indexer's PP gate must
short-circuit before touching the PP group, `allocation`'s DCP gate asks whether
DCP was configured at all, and the CUDA-IPC recycler runs where no group exists).

A new AST ratchet pins both shapes it can see — the direct call and an alias
bound from it in the same function — at 0 and 12 respectively, exempting the
derived APIs and those three sites by name. The alias-form baseline is not zero:
those reads are mostly per-runner fields in model code, and lowering them is the
next slice.

Two fixtures stopped faking config: `test_dllm_fdfo_kv_reuse` rebound
`allocation.get_server_args` to a SimpleNamespace, which silently stops
intercepting the moment a reader migrates; it publishes a real config instead.
Atituiset pushed a commit to Atituiset/sglang that referenced this pull request Sep 10, 2026
…t#33338)

`get_server_args().<field>` reads one process's startup record. Nine sites still
did that for a value that has a namespace: the attention backend (5),
`skip_tokenizer_init` (2), the draft-aware `load_format`, and a chunked-prefill
size in `sglang.kernels`. They now read `get_exec().kernel` / `get_serving()` /
`get_model()` / `get_schedule()`, so they see the resolved value including
post-publish overrides.

The multimodal processor's device selection moves to the instance it was
constructed with rather than to a namespace: `base_gpu_id` differs per worker (the
encode-server DP workers each specialise their own copy), so no process-global
value can stand in for it, and engines sharing a tokenizer process each need their
own. Branch order, the NPU preprocess patches, and the case that leaves "device"
unset are unchanged.

What stays on `get_server_args()` is the derived API — `@property` and method
members computed from several fields plus the HF config
(`mamba_cache_chunk_size`, `get_model_config()`, `enable_mamba_extra_buffer*`) —
plus three config-intent reads of live-shadowed sizes, each of which needs an
answer the live topology property cannot give (the DSA indexer's PP gate must
short-circuit before touching the PP group, `allocation`'s DCP gate asks whether
DCP was configured at all, and the CUDA-IPC recycler runs where no group exists).

A new AST ratchet pins both shapes it can see — the direct call and an alias
bound from it in the same function — at 0 and 12 respectively, exempting the
derived APIs and those three sites by name. The alias-form baseline is not zero:
those reads are mostly per-runner fields in model code, and lowering them is the
next slice.

Two fixtures stopped faking config: `test_dllm_fdfo_kv_reuse` rebound
`allocation.get_server_args` to a SimpleNamespace, which silently stops
intercepting the moment a reader migrates; it publishes a real config instead.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation hicache Hierarchical Caching for SGLang jit-kernel

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant