Skip to content

[Bugfix] Bound the grammar-compile thread pool and expose SGLANG_GRAMMAR_COMPILE_MAX_WORKERS - #32085

Open
anencore94 wants to merge 1 commit into
sgl-project:mainfrom
anencore94:bugfix/grammar-compile-executor-cap
Open

anencore94 wants to merge 1 commit into
sgl-project:mainfrom
anencore94:bugfix/grammar-compile-executor-cap

Conversation

@anencore94

@anencore94 anencore94 commented Jul 22, 2026

Copy link
Copy Markdown

Motivation

Fixes #32084

BaseGrammarBackend creates its grammar-compilation pool as a bare ThreadPoolExecutor(), which defaults to min(32, os.cpu_count() + 4) workers. os.cpu_count() is not cgroup-aware, so inside a container with a CFS quota (e.g. a Kubernetes pod with cpu limit 6–12 on a 172-core GPU node) the pool is sized for the host, not the quota. Each compile can additionally spawn the grammar backend's internal threads (xgrammar defaults to max_threads=8 per compile call), so a burst of distinct guided-JSON schemas can put up to ~256 CPU-bound threads inside the scheduler process, sustain CFS throttling, and stall the decode loop of already-running requests.

We measured this failure class in production on the vLLM side of our fleet (identical root cause, 6-core-limit pod on a 172-core host): CFS throttle ratio ≈ 1.00 sustained for minutes during guided-JSON bursts, inter-token latency collapsing 7–9 ms → 68–93 ms with the GPU nearly idle. Details: vllm-project/vllm#49460 (companion fix: vllm-project/vllm#49461).

Modifications

  • python/sglang/srt/constrained/base_grammar_backend.py: add get_grammar_compile_max_workers() — sizes the pool as max(1, min(cpu_count // 2, 8)), with SGLANG_GRAMMAR_COMPILE_MAX_WORKERS (0 = auto) as an operator override — and use it for BaseGrammarBackend.executor.
  • python/sglang/srt/environ.py: add SGLANG_GRAMMAR_COMPILE_MAX_WORKERS = EnvInt(0).
  • test/registered/unit/constrained/test_base_grammar_backend.py: add TestGrammarCompileMaxWorkers — cap table across CPU counts (1 → 172), default-to-host-count path, env override, 0 = auto, ≥1 floor, and that BaseGrammarBackend.executor is bounded.

Behavior is unchanged on small hosts (≤16 cores: same or nearly same worker count as before); on many-core hosts the pool shrinks from 32 to 8, which also reduces the duplicate-compile fanout noted in #32084.

Accuracy Tests

Not applicable — no change to model outputs; only thread-pool sizing.

Speed Tests and Profiling

No change to the compile path itself; per-compile parallelism (xgrammar max_threads=8) is untouched. On CFS-limited containers this strictly reduces oversubscription; on unlimited many-core hosts, peak concurrent compiles drop from 32 to 8 (override available via SGLANG_GRAMMAR_COMPILE_MAX_WORKERS).

Checklist

🤖 Generated with Claude Code


CI States

Latest PR Test (Base): ❌ Run #32624797750
Latest PR Test (Extra): ❌ Run #32624797604
Latest PR Test (AMD ROCm 7.2): ❌ Run #32624797871

@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.

@anencore94
anencore94 marked this pull request as ready for review July 22, 2026 15:27
@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.

@anencore94
anencore94 force-pushed the bugfix/grammar-compile-executor-cap branch from 9a7b898 to 0475995 Compare July 24, 2026 15:45
@anencore94

Copy link
Copy Markdown
Author

@hnyls2002 — small self-contained fix for a production incident: BaseGrammarBackend's grammar-compile ThreadPoolExecutor uses the default (min(32, cpu_count + 4)) sized by host CPU count, which under a container CPU limit oversubscribes the quota and stalls the scheduler's decode loop (details and RCA in #32084). It caps the pool to max(1, min(cpu_count // 2, 8)), adds SGLANG_GRAMMAR_COMPILE_MAX_WORKERS as an override, and includes a unit test.

I've just rebased onto main — the earlier red lint was the unrelated pre-existing qwen3_5.py F821 (get_server_args), already fixed on main. Could you take a look and apply run-ci when you get a chance? Thanks!

…MAR_COMPILE_MAX_WORKERS

BaseGrammarBackend creates its grammar-compilation pool as a bare
ThreadPoolExecutor(), which defaults to min(32, os.cpu_count() + 4)
workers. os.cpu_count() is not cgroup-aware, so inside a container with
a CFS quota (e.g. a Kubernetes pod with cpu limit 6 on a 172-core host)
the pool is sized for the host, not the quota. Since each compile can
additionally spawn the backend's internal threads (xgrammar defaults to
max_threads=8 per compile), a burst of distinct schemas can put dozens
to hundreds of CPU-bound threads inside the scheduler process, sustain
CFS throttling, and stall the decode loop of already-running requests.

Cap the pool at min(cpu_count // 2, 8) and expose
SGLANG_GRAMMAR_COMPILE_MAX_WORKERS as an operator override (0 = auto).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Jaeyeon Kim <anencore94@gmail.com>
@anencore94
anencore94 force-pushed the bugfix/grammar-compile-executor-cap branch from 7187249 to 0b89cd2 Compare August 23, 2026 07:08
@anencore94

Copy link
Copy Markdown
Author

Rebased onto the latest main and resolved the conflicts.

On the red checks: they're all pr-gate and pr-test-*-finish, which fail because this PR has no run-ci label — no test has actually run. I'm not listed in .github/CI_PERMISSIONS.json, so /tag-run-ci-label is a no-op when I post it. Could a maintainer add run-ci (or run /tag-and-rerun-ci)? I'll fix whatever it surfaces.

@hnyls2002 @DarkSharpness @JustinTong0323 — recap of the fix: BaseGrammarBackend's compile ThreadPoolExecutor uses the default min(32, cpu_count + 4), which is derived from the host CPU count and is not cgroup-aware. Under a container CPU limit, the compile workers oversubscribe the quota, and a burst of structured-output requests then stalls the scheduler's decode loop. The PR caps the pool relative to the effective limit and exposes SGLANG_GRAMMAR_COMPILE_MAX_WORKERS as an override. Background in #32084.

This is the same class of fix as vllm-project/vllm#49461, which is carrying the ready label on that side. Small and self-contained — PTAL.

@anencore94

Copy link
Copy Markdown
Author

Following the merge process in MAINTAINER.md: this PR never got past step 2. No Merge Oncall was ever assigned (the assignee list is empty), and without step 3 the run-ci label was never applied — so the red checks are pr-gate refusing to start, and no test has ever actually run on this branch.

Step 4 says the author may ping other related Merge Oncalls when the assigned one is unresponsive, so per the Scheduler area: @merrymercy @hnyls2002 @cctry — could one of you assign an oncall and add run-ci (or /tag-and-rerun-ci)? I'm not in .github/CI_PERMISSIONS.json, so I can't trigger it myself.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

1 participant