minke.yu/sglang
1
0
Fork 0
Code Issues Pull Requests Actions 12 Packages Projects Releases Wiki Activity

[Config] One writer for the declaration stash; no exception to the write seal (#38752)

Browse Source
This commit is contained in:
Cheng Wan
2026-09-09 19:22:06 -07:00
committed by GitHub
parent 9a1b1d2d5e
commit 53dc77ff4e
19 changed files with 333 additions and 420 deletions
Download Patch File Download Diff File Expand all files Collapse all files
test/registered/unit/server_args/test_model_config_cache.py
+9 -6
View File
@@ -148,16 +148,19 @@ class TestTheModelConfigCache(CustomTestCase):
second_checkpoint = self._checkpoint()
server_args = self._resolved(model_path=first_checkpoint)
copy_ = server_args.replace_resolved(
self.assertEqual(model_config_of(server_args).model_path, first_checkpoint)
# Declaring a new `model_path` moves what the memo is keyed on, so the
# next read rebuilds rather than handing back a configuration that
# describes the previous checkpoint.
declare_resolution(
server_args,
"test_the_cache_refills_on_a_resolved_record",
model_path=second_checkpoint,
)
rebuilt = model_config_of(copy_)
rebuilt = model_config_of(server_args)
self.assertEqual(rebuilt.model_path, second_checkpoint)
self.assertIs(model_config_of(copy_), rebuilt)
# The parent keeps the configuration it resolved with.
self.assertEqual(model_config_of(server_args).model_path, first_checkpoint)
self.assertIs(model_config_of(server_args), rebuilt)
def test_a_supplied_configuration_is_handed_back(self):
"""A configuration nothing in here built carries no key, so nothing
test/registered/unit/server_args/test_resolution_declarations.py
+12 -12
View File
@@ -436,7 +436,7 @@ class TestResolutionDeclarations(CustomTestCase):
The parser detection and the LoRA normalization run at launcher stage --
they need a tokenizer, a chat template, an adapter directory -- and they
declare through `declare_late_resolution`. The declaration is the only
declare through `declare_resolution`. The declaration is the only
home for what they decide: the record keeps `--reasoning-parser auto`,
and the bags a process publishes carry the detected parser.
@@ -444,14 +444,12 @@ class TestResolutionDeclarations(CustomTestCase):
so its `resolve_once` re-runs and re-snapshots the raw input from
already-late-resolved fields, which hides exactly this.
"""
from sglang.srt.arg_groups.overrides import declare_late_resolution
from sglang.srt.arg_groups.overrides import declare_resolution
from sglang.srt.runtime_context import get_serving, publish, reset_context
server_args = self._resolve({"reasoning_parser": "auto"})
self.addCleanup(reset_context)
declare_late_resolution(
server_args, "template-detection", reasoning_parser="qwen3"
)
declare_resolution(server_args, "template-detection", reasoning_parser="qwen3")
self.assertEqual(
resolution_result(server_args, "reasoning_parser"),
"qwen3",
@@ -469,10 +467,10 @@ class TestResolutionDeclarations(CustomTestCase):
def test_pre_engine_late_resolution_reaches_the_projection(self):
"""A launcher declaration survives the engine's first resolution pass."""
from sglang.srt.arg_groups.overrides import declare_late_resolution
from sglang.srt.arg_groups.overrides import declare_resolution
server_args = ServerArgs(model_path="dummy")
declare_late_resolution(
declare_resolution(
server_args,
"launcher",
enable_forward_pass_metrics=True,
@@ -695,11 +693,13 @@ class TestResolutionDeclarations(CustomTestCase):
server_args.attention_backend = "triton"
server_args.schedule_conservativeness = 0.5
from sglang.srt.arg_groups import pipeline as pipeline_module
from sglang.srt import platforms as platforms_module
# The write capture runs in the dispatcher, so that is the namespace the
# plugin has to be installed in.
with unittest.mock.patch.object(pipeline_module, "current_platform", _Plugin()):
# `handle_platform_defaults` imports `current_platform` when it runs, so
# the platform module is the namespace to install the plugin in.
with unittest.mock.patch.object(
platforms_module, "current_platform", _Plugin()
):
server_args = self._resolve({})
self.assertEqual(
(
@@ -738,7 +738,7 @@ class TestDeclaredValuesAreNotEditedLater(CustomTestCase):
The property is about the stash, so the seam is the stash: a list that
snapshots on append. Every declaration path -- `declare_resolution`,
`declare_late_resolution`, `declare_direct_writes` and the passes --
`declare_resolution`, `record_foreign_defaults` and the passes --
reaches it through `.append`, whatever it was imported as.
"""
recorded = []
test/registered/unit/server_args/test_resolution_is_reproducible.py
+37 -55
View File
@@ -35,7 +35,10 @@ import unittest.mock
import torch
from sglang.srt.arg_groups.overrides import model_config_of, resolution_result
from sglang.srt.arg_groups.overrides import (
declare_resolution,
resolution_result,
)
from sglang.srt.environ import EnvField, envs
from sglang.srt.server_args import ServerArgs
from sglang.srt.utils import is_cuda
@@ -480,15 +483,15 @@ class TestResolutionIsReproducible(_RestoresProcessState, CustomTestCase):
self.assertEqual(getattr(first, "_resolved_overrides", None), first_provenance)
class TestACopyStaysResolved(_RestoresProcessState, CustomTestCase):
class TestALateDeclarationKeepsTheResolution(_RestoresProcessState, CustomTestCase):
"""A resolved record copied with `dataclasses.replace` loses what makes it
resolved, and the next publish resolves it a second time -- over values it
already decided. The Ray paths copy a resolved record to set
`dist_init_addr`, which is how they reach this.
already decided. The Ray paths declare `dist_init_addr` on a record that
has already resolved, which is how they reach this.
"""
def _resolved(self):
config_dir = tempfile.mkdtemp(prefix="replace_resolved_")
config_dir = tempfile.mkdtemp(prefix="late_declaration_")
self.addCleanup(shutil.rmtree, config_dir, ignore_errors=True)
with open(os.path.join(config_dir, "config.json"), "w") as handle:
json.dump(_MINI_CONFIG, handle)
@@ -509,9 +512,9 @@ class TestACopyStaysResolved(_RestoresProcessState, CustomTestCase):
`dataclasses.replace` copies the fields, so a bare copy re-runs
resolution over the *same input* the parent got -- the DP-attention
halving and the conservativeness scaling apply once. `replace_resolved`
buys something else: it carries the parent's declarations and its
`model_config`, so the copy answers without resolving at all.
halving and the conservativeness scaling apply once. This is why the Ray
paths declare on the record they were handed instead of copying it: the
record arrives resolved, and a copy would throw that away.
"""
parent = self._resolved()
bare = dataclasses.replace(parent, dist_init_addr="1.2.3.4:5000")
@@ -537,56 +540,34 @@ class TestACopyStaysResolved(_RestoresProcessState, CustomTestCase):
"reading its own output again",
)
def test_replace_resolved_keeps_the_parents_resolution(self):
parent = self._resolved()
copy_ = parent.replace_resolved("ray.test", dist_init_addr="1.2.3.4:5000")
self.assertTrue(getattr(copy_, "_resolution_finished", False))
drifted = {
field.name: (getattr(parent, field.name), getattr(copy_, field.name))
for field in dataclasses.fields(parent)
if field.name != "dist_init_addr"
and getattr(parent, field.name) != getattr(copy_, field.name)
}
self.assertEqual(
drifted,
{},
f"the copy differs from its parent beyond the change: {drifted}",
)
self.assertEqual(copy_.dist_init_addr, "1.2.3.4:5000")
def test_a_late_change_leaves_the_rest_of_the_resolution_alone(self):
"""What the Ray paths do: declare one field on a record that has already
resolved, then hand it to the process that will publish it.
def test_the_copy_carries_what_resolution_left_on_the_record(self):
"""Not just the stash and the flag.
`model_config_of()` memoizes on the record, and that cache is filled
during resolution. A copy that is marked resolved but arrives without it
cannot fill it -- the read-only guard refuses the cache write -- so the
first `model_config_of()` raises. That is what killed the Ray
schedulers, and it is why the carry is enumerated from the instance
rather than from a list of names.
The record stays resolved, so nothing re-derives; the field stays the
operator's input, because resolution does not write fields; and the
decision is what `resolution_result` answers.
"""
parent = self._resolved()
copy_ = parent.replace_resolved("ray.test", dist_init_addr="1.2.3.4:5000")
fields = {field.name for field in dataclasses.fields(parent)}
missing = sorted(
name
for name in vars(parent)
if name not in fields and name not in vars(copy_)
)
self.assertEqual(
missing,
[],
f"the copy did not carry what resolution left on the record: {missing}",
)
self.assertIsNotNone(model_config_of(copy_))
# Containers are copied, so the copy's declaration stays with it.
self.assertEqual(
len(parent._resolved_overrides) + 1, len(copy_._resolved_overrides)
declare_resolution(parent, "ray.test", dist_init_addr="1.2.3.4:5000")
self.assertTrue(getattr(parent, "_resolution_finished", False))
self.assertIsNone(
parent.dist_init_addr,
"the declaration wrote the field; the record is the operator's input",
)
self.assertEqual(resolution_result(parent, "dist_init_addr"), "1.2.3.4:5000")
def test_the_change_reaches_the_bags(self):
"""The projection reads the raw snapshot plus the declarations, so a
change the copy only wrote to the field would publish the parent's raw
value."""
change written only to the field would publish the raw value instead.
This is the Ray hop: the actor receives the record by pickle, declares
its own `dist_init_addr`, and publishes. Nothing else may move --
publishing must not re-run resolution.
"""
import pickle
from sglang.srt.runtime_context import (
get_parallel,
get_schedule,
@@ -595,16 +576,17 @@ class TestACopyStaysResolved(_RestoresProcessState, CustomTestCase):
)
parent = self._resolved()
copy_ = parent.replace_resolved("ray.test", dist_init_addr="1.2.3.4:5000")
arrived = pickle.loads(pickle.dumps(parent))
declare_resolution(arrived, "ray.test", dist_init_addr="1.2.3.4:5000")
self.addCleanup(reset_context)
reset_context()
publish(copy_, role="scheduler")
publish(arrived, role="scheduler")
self.assertEqual(get_parallel().dist_init_addr, "1.2.3.4:5000")
self.assertEqual(
get_schedule().chunked_prefill_size,
resolution_result(parent, "chunked_prefill_size"),
"publishing the copy re-ran resolution; the bag disagrees with what "
"the parent's resolution decided",
"publishing re-ran resolution; the bag disagrees with what the "
"parent's resolution decided",
)
test/registered/unit/server_args/test_resolution_reads_the_declarations.py
+2 -3
View File
@@ -86,8 +86,7 @@ def _field_reads(fn, holders):
_DECLARERS = frozenset(
{
"declare_resolution",
"declare_late_resolution",
"declare_direct_writes",
"record_foreign_defaults",
}
)
@@ -271,7 +270,7 @@ def _record_aliases(function):
aliases.add(target.id)
elif (
isinstance(func, ast.Attribute)
and func.attr in ("from_cli_args", "replace_resolved")
and func.attr == "from_cli_args"
and isinstance(func.value, ast.Name)
and func.value.id == "ServerArgs"
):
test/registered/unit/server_args/test_server_args.py
+23 -17
View File
@@ -2886,17 +2886,32 @@ class TestTheInputIsSealedDuringResolution(CustomTestCase):
with self.assertRaisesRegex(AttributeError, "after resolution"):
server_args.tp_size = 4
def test_the_named_exception_lifts_it(self):
"""`declare_direct_writes` hands the record to an out-of-tree platform
plugin that sets fields on it; that is the only channel."""
from sglang.srt.server_args import record_writable
def test_it_has_no_exception(self):
"""A resolver from outside this tree assigns fields -- an interface this
tree does not own -- and it still does not reach the record.
`record_foreign_defaults` hands it a stand-in: the assignment is
captured and declared, the field keeps the operator's input, and the
seal stays armed for the whole call. There used to be a named lift for
this, which made the record the one thing resolution could write.
"""
from sglang.srt.arg_groups.overrides import (
record_foreign_defaults,
resolution_result,
)
server_args = ServerArgs(model_path="dummy", device="cuda")
object.__setattr__(server_args, "_input_frozen", True)
with record_writable(server_args):
server_args.tp_size = 4
self.assertEqual(server_args.tp_size, 4)
# and it goes back on afterwards
def foreign(config):
# What a plugin does: read what is decided, assign a default.
assert config.tp_size == 1
config.tp_size = 4
record_foreign_defaults(server_args, "platform:probe", foreign)
self.assertEqual(resolution_result(server_args, "tp_size"), 4)
self.assertEqual(server_args.tp_size, 1, "the record is the input")
with self.assertRaisesRegex(AttributeError, "during resolution"):
server_args.tp_size = 8
@@ -2942,15 +2957,6 @@ class TestLaunchCommand(CustomTestCase):
server_args.launch_command,
)
def test_a_copy_keeps_it(self):
"""`replace_resolved` is how the Ray paths rewrite `dist_init_addr`;
the copy was launched by whatever launched its parent."""
server_args = prepare_server_args(["--model-path", "/tmp/x"])
self.assertEqual(
server_args.replace_resolved("test").launch_command,
server_args.launch_command,
)
def test_it_is_not_a_config_field(self):
"""It describes how the configuration was asked for, so it is not part
of the configuration: no CLI flag, no namespace, not in the bags."""
test/registered/unit/test_chain_read_ratchet.py
+2 -39
View File
@@ -40,7 +40,7 @@ _OWNERS = ("server_args.py", "runtime_context.py", "arg_groups/")
# startup default wherever it is written, and `benchmark/` ships too.
_READS_SCANNED = _PACKAGE
_DECLARERS = ("declare_resolution", "declare_late_resolution")
_DECLARERS = ("declare_resolution",)
def _declared_by_keyword():
@@ -239,27 +239,6 @@ def _declared_by_registry_and_passes():
return fields
def _declared_by_late_resolution():
"""Keywords of `declare_late_resolution(record, ...)`, the late spelling.
The fields sit at the call sites rather than in the declarer, so a scan
that only knew the declarer's own definition would find none of them.
"""
# The record plus `arg_groups/`: a hook calls it on the record it was
# handed, so scanning the record's file alone finds nothing.
sources = [_SRT / "server_args.py", *sorted((_SRT / "arg_groups").rglob("*.py"))]
fields = set()
for source in sources:
for node in ast.walk(ast.parse(source.read_text(encoding="utf-8-sig"))):
if (
isinstance(node, ast.Call)
and isinstance(node.func, ast.Name)
and node.func.id == "declare_late_resolution"
):
fields |= {keyword.arg for keyword in node.keywords if keyword.arg}
return fields
def _written_after_publish():
"""Fields the runtime overrides once the bags exist.
@@ -286,7 +265,6 @@ def _resolution_written():
return (
_declared_by_keyword()
| _declared_by_registry_and_passes()
| _declared_by_late_resolution()
| _written_after_publish()
)
@@ -561,7 +539,6 @@ class TestNoChainReadsOfResolvedConfig(CustomTestCase):
by_keyword = _declared_by_keyword()
by_data = _declared_by_registry_and_passes()
by_late = _declared_by_late_resolution()
self.assertGreater(
len(by_keyword),
@@ -586,24 +563,10 @@ class TestNoChainReadsOfResolvedConfig(CustomTestCase):
f"{len(overrides.POST_PROCESS_PASSES)} passes; the scan of the "
"dict-key channel broke",
)
self.assertGreaterEqual(
len(by_late),
3,
f"only {len(by_late)} fields are declared late; the "
"`declare_late_resolution` keyword scan broke",
)
# The data channel is not the keyword scan's subset: if it became one,
# that scan would be doing all the work and a regression here would be
# invisible. The late channel *is* a subset, and deliberately so --
# `declare_late_resolution` is a keyword declarer like the others now
# that the record hosts no forwarding member, so its own floor above is
# what pins it.
# invisible.
self.assertTrue(by_data - by_keyword, "the data channel adds nothing")
self.assertTrue(
by_late <= by_keyword,
"late resolution declares outside the keyword channel; it is the "
"same spelling, so the two cannot disagree",
)
def test_nothing_reads_a_resolved_field_off_a_borrowed_record(self):
found = _chain_reads(_resolution_written())
test/registered/unit/test_supplied_instance_exposure_ratchet.py
+28 -18
View File
@@ -112,7 +112,7 @@ _MATRIX = (
{"enable_mis": True, "attention_backend": "flashinfer"},
)
# `declare_late_resolution` call sites whose keyword expansion is built
# `declare_resolution` call sites whose keyword expansion is built
# dynamically; the written fields are spelled out here and drift-guarded.
_LATE_RESOLUTION_DYNAMIC_SITES = {
"parser/template_detection.py": frozenset({"reasoning_parser", "tool_call_parser"}),
@@ -341,10 +341,10 @@ class TestSuppliedInstanceExposure(CustomTestCase):
union does not depend on matrix order; and the ambient CI marker is
cleared, so a runner's identity cannot leak into the measurement --
the CI-conditioned writes come from `_ENV_MATRIX`'s explicit entry.
Late resolution counts too: `declare_late_resolution` writers run at
launcher stage (LoRA normalization, parser auto-detection), so their
target fields are collected statically from the call sites -- they are
resolution writes by definition, just staged after `__post_init__`.
Declarers outside `arg_groups/` count too: the parser auto-detection
runs at launcher stage and the NPU helper is called by the pipeline, so
their target fields are collected statically from the call sites --
resolution writes by definition, just not reached by the matrix.
"""
pristine = (dict(os.environ), self._env_field_flags())
written = set()
@@ -383,7 +383,7 @@ class TestSuppliedInstanceExposure(CustomTestCase):
for extra, env in _ENV_MATRIX:
resolve_one(extra, env)
self._restore_process_state(pristine)
written |= self._late_resolution_written_fields()
written |= self._declared_outside_the_pipeline()
written |= self._hook_assignment_targets()
written |= self._record_method_assignment_targets()
written |= self._declarative_override_fields()
@@ -607,22 +607,32 @@ class TestSuppliedInstanceExposure(CustomTestCase):
fields.add(key.value)
return fields
def _late_resolution_written_fields(self) -> set:
"""Fields `declare_late_resolution` writes, collected statically.
def _declared_outside_the_pipeline(self) -> set:
"""Fields declared by a `declare_resolution` caller outside
`arg_groups/`, collected statically.
These are resolution's launcher-stage writes (they need a tokenizer or
adapter load, so they cannot run in `__post_init__`), which the
construct-and-diff pass above never sees. The keywords at the call
sites are the written fields; an expansion this cannot resolve fails
loudly like the override collector's, except the named dynamic sites
below, whose field sets are spelled out and drift-guarded (each name
must still appear as a constant in the file)."""
Resolution's launcher-stage writes live here -- the auto-detected
parsers need a tokenizer or chat-template load, so they cannot run in
`__post_init__` -- alongside the NPU default helper and the expert-pack
loader, which the pipeline calls the same way. The construct-and-diff
pass above never sees any of them.
`arg_groups/` is deliberately excluded: `_hook_assignment_targets`
covers it exactly, and it resolves the pipeline's own computed
expansions (`record_foreign_defaults` declares a `**` dict this
collector's resolver cannot read). The keywords at the call sites are
the written fields; an expansion this cannot resolve fails loudly like
the override collector's, except the named dynamic sites below, whose
field sets are spelled out and drift-guarded (each name must still
appear as a constant in the file)."""
written = set()
root = _PACKAGE_ROOT
for path in sorted(root.rglob("*.py")):
rel = path.relative_to(root).as_posix()
if rel.startswith("arg_groups/"):
continue
source = path.read_text(encoding="utf-8-sig")
if "declare_late_resolution" not in source:
if "declare_resolution" not in source:
continue
try:
tree = ast.parse(source)
@@ -634,12 +644,12 @@ class TestSuppliedInstanceExposure(CustomTestCase):
and (
(
isinstance(node.func, ast.Name)
and node.func.id == "declare_late_resolution"
and node.func.id == "declare_resolution"
)
or (
isinstance(node.func, ast.Attribute)
and node.func.attr
in ("declare_late_resolution", "_late_resolution")
in ("declare_resolution", "_declare_resolution")
)
)
):