Conversation
There was a problem hiding this comment.
Pull request overview
This PR introduces serving-path performance improvements by adding a native batched draft-tree traversal API for speculative decoding, reusing bounded native thread pools across requests/compilations, and compacting compiled-grammar serialization (v16) to avoid per-rule FSM duplication while validating corruption fail-closed.
Changes:
- Add
root_positionto scalar draft-tree traversal and a newBatchGrammarMatcher.batch_traverse_draft_treeAPI across C++/FFI/Python. - Reuse persistent native worker pools (matcher + compiler), with exception propagation and pool reusability semantics.
- Bump serialization format to
v16with compact per-rule FSM views over one shared complete FSM, plus validation, docs, tests, and a benchmark.
Reviewed changes
Copilot reviewed 21 out of 21 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| tests/python/test_speculative_decoding.py | Adds batch traversal parity/validation/offset tests. |
| tests/python/test_serialization.py | Updates to v16 + corruption/roundtrip tests for compact views. |
| tests/python/test_grammar_compiler.py | Adds concurrent compilation stress test for persistent pool reuse. |
| tests/cpp/test_thread_pool.cc | Adds tests for Wait() rethrow/reuse and ParallelFor exception propagation. |
| tests/cpp/test_grammar_matcher.cc | Adds native regressions for byte_offset, batch failure propagation, pool reuse. |
| python/xgrammar/matcher.py | Exposes root_position and adds Python batch traversal wrapper/docs. |
| include/xgrammar/matcher.h | Extends public C++ API: scalar root_position, batch traverse method. |
| examples/benchmark/README.md | Documents new batch draft-tree benchmark. |
| examples/benchmark/bench_batch_draft_tree.py | Adds reproducible scalar-vs-batch traversal benchmark. |
| docs/using_xgrammar/serialization.md | Documents compact FSM view format and validation behavior. |
| docs/using_xgrammar/engine_integration.md | Documents batch traversal integration + root_position. |
| docs/start/workflow_of_xgrammar.md | Updates compilation threading/pool semantics documentation. |
| cpp/tvm_ffi/tvm_ffi.cc | Wires new APIs through TVM FFI bindings. |
| cpp/support/thread_pool.h | Adds Execute-exception capture + Wait() rethrow/clear; ParallelFor waits before join. |
| cpp/support/json_serializer.h | Bumps global serialization version constant to v16. |
| cpp/grammar.cc | Implements compact per-rule FSM view serialization/deserialization with validation. |
| cpp/grammar_matcher.cc | Adds root_position traversal + batch traversal implementation + persistent pool usage. |
| cpp/grammar_impl.h | Switches Grammar (Impl) JSON (de)serialization to custom functions. |
| cpp/grammar_compiler.cc | Introduces persistent compiler-owned pool and bounded scheduling/drain-on-failure. |
| cpp/fsm.h | Exposes GetIsDFA() accessor used by compact view serialization. |
| cpp/compiled_grammar.cc | Propagates nested deserialization errors instead of ignoring them. |
Comments suppressed due to low confidence (2)
cpp/grammar_matcher.cc:190
- Pointer arithmetic that incorporates
byte_offsetis immediatelyreinterpret_casted toint32_t*without validating alignment. If a caller passes a contiguous DLTensor with an unalignedbyte_offset(or base pointer), this becomes undefined behavior on platforms requiring alignment.
Consider validating alignment (and failing closed) before casting.
auto* data =
static_cast<char*>(token_bitmask.data) + static_cast<std::size_t>(token_bitmask.byte_offset);
return reinterpret_cast<int32_t*>(data) + index * buffer_size;
cpp/grammar_matcher.cc:2219
- The
byte_offset-adjusted pointers for the int64 draft-tree tensors arereinterpret_casted toint64_t*without validating alignment. If a caller passes a view with an unalignedbyte_offset(or base pointer), this is undefined behavior.
Please validate that (uintptr_t(data) + byte_offset) preserves int64_t alignment before casting.
const auto* next_token_data = reinterpret_cast<const int64_t*>(
static_cast<const char*>(retrieve_next_token->data) + retrieve_next_token->byte_offset
);
const auto* next_sibling_data = reinterpret_cast<const int64_t*>(
static_cast<const char*>(retrieve_next_sibling->data) + retrieve_next_sibling->byte_offset
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…ar-performance # Conflicts: # cpp/grammar_matcher.cc # cpp/tvm_ffi/tvm_ffi.cc # include/xgrammar/matcher.h # python/xgrammar/matcher.py
CUDA validation on BlackwellI ran the CUDA-enabled test paths against the current PR head (
Note Triton 3.5.1 bundles CUDA 12.8 Focused CUDA/Triton bitmask coverageTRITON_PTXAS_PATH=/usr/local/cuda/bin/ptxas \
/root/venv-scratch/.venv/bin/python -m pytest \
tests/python/test_token_bitmask_operations.py -qResult: 101 passed, 1 skipped, 0 failed in 134.84s. The single skip is the unrelated MLX backend because MLX is not installed. This exercised the native CUDA, Triton, and Full GPU-enabled Python suiteTRITON_PTXAS_PATH=/usr/local/cuda/bin/ptxas \
/root/venv-scratch/.venv/bin/python -m pytest \
--ignore=3rdparty -m 'not hf_token_required' -q -o addopts=''Result: 3,068 passed, 1 skipped, 622 deselected, 0 failed in 146.13s. The deselected tests require a Hugging Face token; the single skip is again MLX. With CUDA visible, this also exercises the GPU-backed matcher tests and the DLTensor/storage-offset paths affected by this PR. No CUDA-specific correctness issues were found. |
CI follow-up: Python 3.8 matrix fixedThe rerun exposed three failed matrix cells that GitHub's top-level checks view collapsed under duplicate job names:
All three had the same root cause in Commit I reproduced the affected CI configuration locally with Python 3.8.20, PyTorch 2.4.1, Triton 3.0.0, no Hugging Face token, and CUDA hidden to match the CPU-only Actions runners: Additional gates on the current head:
The PR body and all guided-review source links have also been refreshed to the current head. The newly triggered upstream workflows are awaiting external-fork approval ( |
|
@Seven-Streams pushed up some test fixes for the few that were failing. lmk if you need anything else from me |
Summary
This packages three related serving-path improvements: a native batched speculative-decoding traversal API, reusable bounded worker pools for matching and compilation, and compact compiled-grammar serialization. It also hardens DLTensor pointer/offset handling—including sampling-temperature outputs preserved during the upstream rebase—and adds fail-closed validation, regression coverage, integration documentation, and a reproducible synthetic benchmark. The branch is merged through upstream
mainat3fb48bf; the compact wire format is nowv17so it cannot collide with upstream's existingv16temperature format.Guided review
Tip
Short on review time? Follow the numbered path below. It moves from the public contract, through the two concurrency-sensitive implementations, into the cache-format change, and ends at the tests that lock each behavior down. Every source link is pinned to the current PR head, so the code and line anchors below match the rebased implementation exactly.
This change is easiest to understand as one request moving through the system: Python describes a speculative draft tree, the FFI hands tensor views to C++, native workers traverse that tree and write token masks, and the compiler/cache layer makes the grammars feeding those matchers cheaper to build and store.
Review map
flowchart LR A["Python engine integration"] -->|"tokens, tree, roots, output"| B["TVM FFI"] B --> C["BatchGrammarMatcher validation"] C -->|"tensor views only after all checks pass"| D["Persistent native worker pool"] D --> E["Per-request draft traversal"] E --> F["Flattened token-mask rows"] G["GrammarCompiler"] -->|"compiled grammar"| E G --> H["v16: one complete FSM + compact rule views"] H -->|"deserialize and validate"| E1. Begin with the serving-engine contract
The public API change is deliberately small. The scalar matcher gains a
root_positionwhile preserving upstream's timeout and optional temperature-output parameters. The batch matcher gains one operation that accepts all active requests at once. Start with the C++ scalar declaration and the batch method contract, then read the corresponding scalar Python wrapper and batch Python wrapper.In plain language, a caller can now provide:
indices;The result is one completion flag per matcher. The output layout stays flat so C++ can write directly into the engine-owned bitmask without assembling temporary Python objects:
The TVM FFI binding is intentionally mechanical: it forwards already-described tensors and options into the native matcher. That keeps policy and validation in one C++ implementation rather than duplicating it across language boundaries.
Important
The batch API is additive. Existing scalar callers keep their current path, and
root_position=0preserves the previous behavior.Reviewer lens: public API
2. Establish correctness in the scalar primitive first
The batch implementation is built on the same scalar traversal primitive, so the next stop is
TraverseDraftTreeRecursive. It walks the draft tree depth-first, accepts each edge token against the grammar state, emits the allowed-token mask at that node, and rolls the matcher state back before visiting the next sibling. That rollback is what prevents one draft branch from contaminating another.Three correctness details were made explicit before batching:
byte_offset, checks address overflow, and validates alignment. A tensor can be contiguous while still beginning at a nonzero storage offset; ignoring that field reads or writes the wrong part of the allocation.This scalar path is both the compatibility path and the correctness oracle used by the batch parity tests.
Note
“Contiguous” describes strides, not necessarily the beginning of storage. The nonzero-
byte_offsetregression is therefore a real tensor-layout case, not an artificial malformed input.3. Lift that primitive into one native batch operation
Now read the full batch implementation. It has three phases, and their order is part of the safety argument.
3.1 Validate everything before scheduling anything
Lines 1940–2026 reject wrong devices, dtypes, dimensions, row counts, roots, duplicate indices, and output sizes. No worker is launched until every request is known to be valid.
That fail-closed ordering matters: if request 47 is malformed, requests 0–46 must not have partially advanced their matcher states or written partial masks.
3.2 Build zero-copy per-request views
Lines 2028–2073 select each request’s tree, tokens, and output region by rebasing lightweight DLTensor views. The implementation does not copy the draft tensors merely to batch them. It also preserves the original storage offset while calculating each row’s address, which is the batched counterpart of the scalar offset fix.
3.3 Schedule chunks, not individual requests
Lines 2075–2099 divide the selected requests into at most one task per worker. Each task resets its output rows and traverses its contiguous chunk. This makes two formerly separate operations—mask clearing and traversal—one worker pass and avoids queueing one tiny task per request.
The pool itself is created once in the
BatchGrammarMatcherconstructor. The same chunking principle is also used by batched next-token mask generation, so both hot paths avoid per-call thread construction and per-request scheduling overhead.sequenceDiagram participant P as Python caller participant B as Batch matcher participant W as Persistent workers P->>B: tensors + indices + roots + output B->>B: validate the complete batch B->>B: create zero-copy row views B->>W: one chunk per active worker par worker chunks W->>W: clear owned output rows W->>W: traverse owned requests end W-->>B: completion flags / first exception B-->>P: one result per matcherReviewer lens: native batch path
The important invariants are:
4. Verify the persistent worker pool’s failure semantics
Reusing threads is only an optimization if a failed task cannot poison later calls. The central behavior lives in
ThreadPool.Workers catch task exceptions and record the first one while still marking the task complete. Then
Wait()waits for the submitted generation, rethrows that exception on the caller thread, and clears it. Clearing after observation is the key reuse property: the next independent batch can run normally instead of seeing a stale failure.The convenience
ParallelForwaits before joining, so an exception cannot be lost during pool teardown.Warning
This code intentionally does not fall back to serial execution after a worker failure. The operation fails at its boundary, all outstanding work is drained safely, and the reusable pool remains valid for the next call.
The native pool tests exercise repeated waits,
Execute,Submit, worker exceptions, subsequent reuse, andParallelForpropagation. The matcher-native regressions additionally prove that a batch exception reaches the caller and that the same matcher pool works afterward.5. Apply the same bounded-concurrency model to compilation
Grammar compilation used to risk paying thread setup repeatedly, especially when several outer callers compiled grammars concurrently. The compiler now owns one bounded pool for its lifetime, so concurrent callers share the same native capacity instead of multiplying it.
The scheduling flow is:
This produces bounded parallelism without changing the compiled result. The concurrent shared-compiler stress test compares serial and parallel output while many callers reuse one compiler.
6. Read the serialization change as normalization, not compression magic
Upstream already assigned serialization version
v16to sampling-temperature state. This PR changes the optimized grammar layout again, so the rebased branch usesv17rather than reusing an incompatible version tag.The previous representation serialized overlapping FSM data once per rule. Large grammars therefore repeated the same edge auxiliary data hundreds of times. Version 17 stores the normalized structure instead:
CompactRuleFSMViewrecords only a rule's start state, end states, DFA flag, and size metadata.The deserializer checks view count, required complete-FSM presence, start/end bounds, sorted unique end states, and recorded sizes before constructing anything. Nested grammar and adaptive-mask failures are propagated, rather than turning malformed child data into an apparently successful outer object. The format identifier is correspondingly set to v17, and the reason for skipping the already-occupied v16 tag is documented in the serialization guide.
Caution
This is an intentional on-disk cache break. A v16 or older compiled-grammar cache must be cleared during upgrade; silently interpreting two different layouts under one version would be unsafe.
On the current rebased head, the same 256-field/259-rule schema is 31.39× smaller than current upstream
main(96.81% fewer serialized bytes) because the repeated FSM structure is eliminated.7. Finish with the executable contracts
The tests are organized around failure modes a serving-engine reviewer should care about:
ThreadPoolgenerations andParallelForexceptionsSuggested final review checklist
byte_offset.Note
After this guided pass, the benchmark and validation sections below provide the quantitative results and exact commands/suite outcomes. A clearly labeled historical Cursor summary remains at the bottom as an independent map of the pre-rebase implementation.
Relationship to open upstream work
Note
This audit was refreshed on 2026-07-30 against the open
mlc-ai/xgrammarpull requests. This branch is already merged through upstreammaincommit3fb48bf, so PRs merged before that point are incorporated here rather than merely compared below.Executive takeaway
No open upstream PR duplicates either the native batched draft-tree traversal API or the compact shared-FSM
v17representation. One open PR directly overlaps compiler-owned thread pooling, while several others are complementary optimizations or correctness fixes in the same compiler/matcher/FSM surfaces.grammar_compiler.cc,thread_pool.h, pool testsfill_next_token_bitmaskstride-awarebyte_offsetand alignment in draft traversal and temperature outputs. Both are needed.grammar_matcher.cctensor helpersgrammar_compiler.cc, compiler testsgrammar_compiler.ccand compiler concurrency policyDetailed compare/contrast
#438: persistent thread pool
Shared goal. Both changes replace per-compilation pool creation with one pool owned by each
GrammarCompiler, preventing concurrent callers from multiplying native worker pools.Different implementation. #438 introduces a
TaskCounter/callback-oriented pool API and substantially reshapes the existing interface. This PR's compiler path keeps submit/future semantics, compiles workloads smaller than 64 adaptive states inline, and drains all scheduled futures before captured state can unwind. Its pool implementation also defines and tests worker-exception propagation and reuse after a failed generation.Reviewer action. Treat these as alternate implementations of the same pool-lifetime change. Whichever abstraction is retained should preserve the inline threshold, bounded ownership, drain-before-unwind behavior, and exception/recovery tests.
#699: stride-aware token-mask filling
Shared area. Both changes harden DLTensor address calculation around token-mask generation.
Different semantics. #699 uses
strides[0]to locate rows forfill_next_token_bitmaskand accepts safe row-strided layouts. This PR's effective-pointer helper applies basebyte_offset, rejects address overflow/misalignment, and is used by scalar/batched draft traversal and temperature outputs. A stride describes distance between elements;byte_offsetdescribes where the logical tensor begins. Fixing one does not fix the other.Reviewer action. Preserve both fixes. The new batch draft API should continue to reject unsupported non-contiguous traversal tensors explicitly, while existing mask filling can retain #699's broader row-stride support.
#722: compilation and token-mask work reduction
#722 precomputes/shares Earley metadata, deduplicates adaptive-mask work and stored masks, and batch-proves token-prefix subtrees. It reduces how much work is performed and says it preserves the existing serialized format. This PR changes how eager work is scheduled and intentionally introduces the compact
v17format. The designs are conceptually composable despite a large textual conflict surface.After combining them, rerun shared-compiler concurrency stress plus serialized byte equality, matcher parity, corruption rejection, and size measurements; #722's mask deduplication complements rather than replaces the shared-FSM layout.
#731: on-demand token-mask compilation
#731 makes adaptive masks opt-in lazy and materializes them before serialization. This PR keeps eager compilation but makes large eager work persistent-pool-backed. If combined, concurrent first-use mask population must be tested through multiple matchers, and serialization must still materialize every required mask before writing
v17.#753 and #758: newer compiler work
#753 removes a cache-miss conversion round trip and should compose with this PR with only a small
grammar_compiler.ccconflict. #758 introduces additional compiler parallel regions; its work partitioning is complementary, but independent pools could reintroduce oversubscription. Integration should use one explicit concurrency budget and stress simultaneous outer compiler callers.Already incorporated from updated
mainmainafter those improvements.v16, which is why this PR now usesv17and adds root/offset/temperature composition tests.Other open adjacent work
grammar_matcher.cc.What appears unique to this PR
The refreshed open-PR audit did not find another implementation of:
BatchGrammarMatcher.batch_traverse_draft_treeoperation with shared/per-request trees, sparse indices, per-matcher roots, flattened output ownership, and validation-before-scheduling;byte_offsetacross scalar/batched draft traversal and temperature outputs, with overflow and alignment checks;v17serialization using one complete optimized FSM plus compact per-rule views, with structural corruption checks and a current-main comparison; orThe batch API builds on scalar traversal foundations already merged in #490 and #613; it lifts that work into a serving-engine-oriented native batch boundary.
Suggested integration checklist
v17representation, then rerun stress and size tests.v17materialization.Changes
Batched speculative decoding
BatchGrammarMatcher.batch_traverse_draft_treeacross the C++, TVM FFI, and Python APIs.indices, and per-matcherroot_positions.(batch_size * num_nodes, mask_words)bitmask.traverse_draft_treewithroot_positionand correctly honors DLTensorbyte_offsetfor offset-backed tree, mask, and optional temperature tensors.BatchGrammarMatcherinstead of recreating and joining threads on every call.Bounded persistent grammar compilation
GrammarCompiler, shared by concurrentcompile_*calls.Compact, validated serialization
v16tov17;v16is already occupied by sampling-temperature state.[start, ends, is_dfa, edge_num, node_num]view.Tests, docs, and benchmark
ParallelForpropagation.examples/benchmark/bench_batch_draft_tree.py, which is self-contained and downloads no model.Performance results
Current-head rerun at
48a446a, using the checked-in synthetic draft-tree benchmark with 96 requests × 6 nodes, vocabulary 101, and the median of 500 iterations:The thread-count spread is small enough to show normal microbenchmark scheduling noise; the stable result is that one native batch boundary removes roughly half of scalar Python dispatch time.
Compiled serialization for the same 256-field schema and 259 optimized rules, measured from clean Release wheels built from current upstream
mainand the current PR head:edge_aux_datacopiesmain3fb48bfv1648a446av17That is a 96.81% size reduction (31.39× smaller) with exact serialize → deserialize → serialize equality and functional matcher parity. The checked-in shared-compiler stress regression also compiles 32 distinct large schemas through one compiler with eight concurrent callers, compares serial/parallel output, deserializes every result, and verifies pool reuse after idle.
Validation
All results below are from the current source state at
48a446a:Note
48a446achanges test annotations only. The native build, GPU, and serialization results were established on parent3e76ba2, whose implementation sources are byte-for-byte identical; the synthetic benchmark and exact Python 3.8 CI suite were rerun on the current head.pre-commit run --all-files: passruff check python tests examples/benchmark/bench_batch_draft_tree.py: passsm_103a; the test used the host CUDA 13.0 assembler viaTRITON_PTXAS_PATH=/usr/local/cuda/bin/ptxas.v16) versus 725,254 bytes (v17).3e76ba2;v17tests and upgrade documentation included.48a446areplaces runtime-evaluated PEP 585/collections.abcannotations in the upstreammax_charstests with Python 3.8-compatibletypingforms.Compatibility note
The compact representation intentionally uses serialization version
v17. Upstreamv16added sampling-temperature state, and this PR changes the optimized grammar layout again; reusingv16would allow incompatible documents to share one version identifier. Existingv16or older on-disk compiled-grammar caches must therefore be cleared after upgrading, consistent with the documented versioning contract.Warning
Historical automated summary. Cursor generated the block below for commit
91db6bf, before the upstream merge and thev17collision fix. Use the guided review, overlap audit, benchmarks, and validation above for the current head48a446a.Note
High Risk
Touches core matcher/compiler concurrency, serialization format bump to v16 (cache break), and speculative decoding correctness; regressions would affect token masks or compiled grammar loads at scale.
Overview
Title
Batch draft traversal, persistent worker pools, and compact grammar caches (serialization v16)
Intent
Serving workloads need speculative draft-tree mask generation and grammar compilation to scale without per-call thread churn or multi-megabyte serialized caches. This PR packages those paths behind native batch APIs, shared bounded pools, and a compact FSM representation while failing closed on corrupt serialized data.
Details
BatchGrammarMatcher.batch_traverse_draft_tree(C++, FFI, Python) traverses one tree per matcher, supports shared or per-request 2D trees, sparseindices, per-matcherroot_positions, and flattened(batch_size × num_nodes)mask rows; scalartraverse_draft_treegainsroot_positionand honors DLTensorbyte_offset.BatchFillNextTokenBitmaskreuses one pool perBatchGrammarMatcher, chunks work per worker, and propagates worker exceptions; newBatchTraverseDraftTreevalidates contiguity, dtypes, shapes, unique indices, and roots before scheduling.GrammarCompilerkeeps one persistentThreadPool; small grammars inline adaptive mask work below a state threshold; large work usesSubmitwith drain-on-exception semantics.complete_fsmplus per-rule compact views[start, ends, is_dfa, edge_num, node_num]; deserialization validates counts, bounds, sorted unique ends, and nested grammar/mask errors;CompiledGrammardeserialization checks nested failures.Executetasks record first exception;Wait()rethrows and clears;ParallelForcallsWait()beforeJoin().bench_batch_draft_tree.py.Roadmap for Reviewers
include/xgrammar/matcher.handpython/xgrammar/matcher.pyfor the public API surface (root_position,batch_traverse_draft_tree).cpp/grammar_matcher.cc— core traversal, bitmask offsets, batch validation, and pool chunking (large file; focus onTraverseDraftTreeRecursive,BatchTraverseDraftTree,BatchFillNextTokenBitmask).cpp/grammar_compiler.cc— persistent pool,kMinParallelStates, and exception-safe scheduling.cpp/grammar.cc+cpp/grammar_impl.h— compact serialize/deserialize and view reconstruction overcomplete_fsm.cpp/support/thread_pool.h— exception propagation and pool reuse (small, high leverage).cpp/compiled_grammar.cc— propagated deserialize errors (quick skim).cpp/tvm_ffi/tvm_ffi.cc— FFI bindings (mostly mechanical).tests/python/test_speculative_decoding.py,tests/python/test_serialization.py,tests/cpp/test_grammar_matcher.cc— skim for coverage intent.Diagrams
Reviewed by Cursor Bugbot for commit 91db6bf. Bugbot is set up for automated code reviews on this repo. Configure here.