Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces significant performance and memory optimizations to the grammar compiler and JSON schema converter. Key changes include parsing EBNF rules directly into a GrammarBuilder to avoid materializing a combined script, optimizing TagDispatch rules using a new byte-oriented Aho-Corasick matcher with sparse candidate matching, and reducing memory allocations during FSM sequence construction. A high-severity issue was identified in the Aho-Corasick trie builder, where holding a reference to a vector element while calling emplace_back can lead to a dangling reference and undefined behavior if reallocation occurs.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| int32_t& next_state = nodes_[state].next[byte]; | ||
| if (next_state == -1) { | ||
| next_state = static_cast<int32_t>(nodes_.size()); | ||
| nodes_.emplace_back(); | ||
| } | ||
| state = next_state; |
There was a problem hiding this comment.
Holding a reference to nodes_[state].next[byte] (via next_state) while calling nodes_.emplace_back() is unsafe. If emplace_back() triggers a reallocation of the nodes_ vector, the reference next_state becomes dangling, leading to undefined behavior when it is subsequently read or written to. Although reserve() is called beforehand, relying on it to prevent dangling references is error-prone and discouraged. It is safer to store the state index by value and perform the lookup/assignment after emplace_back().
int32_t next_state = nodes_[state].next[byte];
if (next_state == -1) {
next_state = static_cast<int32_t>(nodes_.size());
nodes_.emplace_back();
nodes_[state].next[byte] = next_state;
}
state = next_state;There was a problem hiding this comment.
Pull request overview
Note
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
This PR reduces cold-start grammar compilation overhead by avoiding repeated substring scans, avoiding dense per-token work during structural-tag dispatch, and parsing generated JSON-schema EBNF per-rule into a shared GrammarBuilder to reduce intermediate materialization.
Changes:
- Introduces a byte-oriented Aho–Corasick automaton and uses it for TagDispatch token preclassification.
- Adds a per-rule EBNF parsing path (
ParseEBNFRules) and a JSON Schema → Grammar conversion path that bypasses the combined EBNF script. - Optimizes FSM sequence concatenation / optimizer no-ops and adds focused C++ and Python regression tests.
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/python/test_json_schema_converter.py | Adds a regression ensuring direct JSON Schema→Grammar matches the text-EBNF path. |
| tests/python/test_grammar_matcher_structural_tag.py | Adds coverage ensuring sparse TagDispatch candidate masks match direct parsing behavior. |
| tests/cpp/test_parser.cc | Adds a regression comparing per-rule parsing vs parsing a combined EBNF script. |
| tests/cpp/test_aho_corasick.cc | Adds unit tests for failure links, terminal propagation, offsets, and exhaustive equivalence. |
| cpp/support/aho_corasick.h | Adds the Aho–Corasick matcher interface. |
| cpp/support/aho_corasick.cc | Implements the Aho–Corasick matcher with resolved transitions and terminal propagation. |
| cpp/json_schema_converter.h | Exposes ConvertToGrammar and JSONSchemaToGrammar APIs. |
| cpp/json_schema_converter.cc | Implements per-rule generation + direct Grammar conversion from JSON schema. |
| cpp/grammar_parser.h | Exposes ParseEBNFRules API. |
| cpp/grammar_parser.cc | Implements per-rule parsing into a shared builder to avoid combined script/token stream. |
| cpp/grammar_functor.cc | Optimizes used-rule discovery, streams FSM sequence concatenation, and skips no-op optimizer passes. |
| cpp/grammar_compiler.cc | Adds TagDispatch token optimization with Aho–Corasick + sparse candidate traversal. |
| cpp/grammar.cc | Switches default JSON Schema frontend to the direct Grammar conversion path when not printing EBNF. |
| cpp/ebnf_script_creator.h | Exposes generated rules directly to avoid materializing the combined EBNF script. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| std::vector<int> state_mapping; | ||
| for (int32_t sequence_id : expr) { | ||
| const auto& element_expr = grammar->GetGrammarExpr(sequence_id); | ||
| int element_start = result_fsm.NumStates(); | ||
| int element_end = -1; | ||
| switch (element_expr.type) { | ||
| case ExprType::kByteString: { | ||
| int current_state = result_fsm.AddState(); | ||
| for (int32_t byte : element_expr) { | ||
| int next_state = result_fsm.AddState(); | ||
| result_fsm.AddEdge( | ||
| current_state, next_state, static_cast<uint8_t>(byte), static_cast<uint8_t>(byte) | ||
| ); | ||
| current_state = next_state; | ||
| } | ||
| element_end = current_state; | ||
| break; | ||
| } | ||
| case ExprType::kRuleRef: { | ||
| result_fsm.AddState(); | ||
| element_end = result_fsm.AddState(); | ||
| result_fsm.AddRuleEdge(element_start, element_end, element_expr[0]); | ||
| break; | ||
| } | ||
| case ExprType::kRepeat: { | ||
| result_fsm.AddState(); | ||
| element_end = result_fsm.AddState(); | ||
| result_fsm.AddRepeatEdge( | ||
| element_start, element_end, element_expr[0], element_expr[1], element_expr[2] | ||
| ); | ||
| break; | ||
| } | ||
| case ExprType::kToken: | ||
| case ExprType::kExcludeToken: { | ||
| result_fsm.AddState(); | ||
| element_end = result_fsm.AddState(); | ||
| std::vector<int32_t> token_ids(element_expr.begin(), element_expr.end()); | ||
| if (element_expr.type == ExprType::kToken) { | ||
| result_fsm.AddTokenEdge(element_start, element_end, token_ids); | ||
| } else { | ||
| result_fsm.AddExcludeTokenEdge(element_start, element_end, token_ids); | ||
| } | ||
| break; | ||
| } | ||
| case ExprType::kCharacterClass: | ||
| case ExprType::kCharacterClassStar: { | ||
| auto element_fsm = CharacterClass(element_expr); | ||
| result_fsm.AddFSM(element_fsm.GetFsm(), &state_mapping); | ||
| element_start = state_mapping[element_fsm.GetStart()]; | ||
| XGRAMMAR_DCHECK(element_fsm.GetEnds().size() == 1); | ||
| element_end = state_mapping[element_fsm.GetEnds()[0]]; | ||
| break; | ||
| } |
| auto new_rule = ParseRule(); | ||
| XGRAMMAR_DCHECK(new_rule.name == rule_name); | ||
| XGRAMMAR_CHECK(Peek().type == TokenType::EndOfFile) | ||
| << "Unexpected trailing tokens in generated rule \"" << rule_name << "\""; | ||
| builder_.UpdateRuleBody(new_rule.name, new_rule.body_expr_id); | ||
| builder_.UpdateLookaheadAssertion(new_rule.name, new_rule.lookahead_assertion_id); |
| @@ -0,0 +1,49 @@ | |||
| /*! | |||
| * Copyright (c) 2026 by Contributors | |||
| * \file xgrammar/support/aho_corasick.h | |||
00b9bdc to
cf57b14
Compare
## Summary This PR speeds up finite-state machine (FSM) construction during grammar compilation by building grammar fragments directly in one shared target FSM. It started from the two sequence-building optimizations split out of #711, then generalized the same construction path to arbitrarily nested grammar expressions. Before this change, sequence construction created one small `FSMWithStartEnd` per element, retained the objects in a list, and copied them into a combined FSM through `Concat`. Choice construction repeated the pattern: it built one FSM per branch, retained all branch FSMs, and copied them again through `Union`. Large grammars therefore created and destroyed many short-lived vectors and FSM objects. ### Implementation - `GrammarFSMBuilderImpl` holds the target FSM being constructed. - `BuildExpression` recursively dispatches any expression shape, so FSM construction no longer depends on the normalized `rule -> choices -> sequence -> element` layout. - Separate leaf builders handle byte strings, rule references, character classes (including negative and starred classes), repetitions, token edges, and exclude-token edges directly in the shared target FSM. - The first sequence child uses the sequence start directly. Each later child gets an isolated start state, and every end state of the previous child is connected to it with an epsilon (empty-input) edge. This supports fragments with multiple end states without assuming a single end. - Multiple choice branches are streamed into the same target FSM. Their end-state IDs are collected as the accepting-state set; state nodes are not forcibly merged during construction. - Regex, tag-dispatch, and token-tag-dispatch expressions retain their specialized builders, then append the resulting FSM through the same recursive expression path. - End-state vectors are reused while walking a sequence. There is no per-element sorting or deduplication; the final `FSMWithStartEnd` construction keeps the existing normalization behavior. - Existing standalone entry points such as `ByteString` and the original `JSONStringForbiddenChars` API remain unchanged. The construction order deliberately preserves the old result: state numbering, edge insertion order, auxiliary edge data, start state, and end states remain unchanged. ## Correctness testing `tests/python/test_fsm_sequence_build.py` contains 24 real-workload cases with no mocks: - Exhaustive comparison against equivalent Python regular expressions over bounded alphabets. - Byte strings, positive, negative, and starred character classes, rule references, empty alternatives, repetitions, and nested alternatives. - Arbitrarily nested sequence and choice expressions and nested dispatch expressions built without running the structure normalizer first. - UTF-8 multi-byte strings and Unicode character classes. - A 128-element mixed sequence with truncation, extension, and mutation rejection checks. - Recursive rule references at depths up to 32. - `Token()` and `ExcludeToken()` edges with a real custom vocabulary. - Token-bitmask consistency against forked matcher acceptance at every generation step. `tests/python/test_fsm_structure_stability.py` adds 17 exact-layout cases: 14 Extended Backus-Naur Form grammars, two JSON Schemas, and one structural-tag grammar. Each case checks a SHA-256 digest of the printed FSM bytes, so changes to state numbering, edge order, start and end sets, or complete-FSM layout fail the test. Verification on the builder-only branch, with the current native library loaded explicitly: - 427 selected Python grammar, compiler, parser, matcher, and new FSM tests passed. - All 61 C++ tests passed. - Main (`ff923ddf`) and the builder branch compiled the same 17-grammar corpus. The printed FSMs and complete serialized grammars were byte-identical across all 177,330 output bytes. - The isolated benchmark produced the same state-count checksum for both revisions. ## Performance Measured on Linux x86-64 on the same shared host with identical `RelWithDebInfo` builds. Compared revisions: - `ff923ddf`: main before the FSM construction optimizations. - `23746d7f`: this builder-only PR. ### Isolated FSM construction A standalone C++ benchmark calls `GrammarFSMBuilder::Sequence`, `GrammarFSMBuilder::Choices`, and `GrammarFSMBuilder::Apply` directly, excluding parsing, tokenizer processing, and unrelated optimizer passes. Each revision ran in three independent processes in interleaved order. Every process used 3 warm-up iterations followed by 15 measured iterations, giving 45 measured samples per revision. Values below are pooled medians. - Long sequence, 10,000 literal and character-class pairs: **12.656 -> 5.518 ms** (**56.4% reduction, 2.29x faster**). - One choice with 8,000 branches: **74.397 -> 66.402 ms** (**10.7% reduction, 1.12x faster**). - Full FSM builder on an 8,000-rule ring grammar: **46.813 -> 37.766 ms** (**19.3% reduction, 1.24x faster**). ### Grammar optimizer stage This measures the full grammar optimizer stage, which contains FSM construction plus byte-string fusion, rule inlining, repetition expansion, dead-code elimination, and grammar analyses. Results are pooled medians from two independent runs of five repeats each: - Flat JSON Schema, 50,000 string fields: **1289.3 -> 1157.6 ms** (**10.2% reduction**). - Flat JSON Schema, 20,000 string fields: **486.7 -> 421.0 ms** (**13.5% reduction**). - Ring Extended Backus-Naur Form grammar, 4,000 rules: **40.6 -> 34.1 ms** (**16.0% reduction**). - Long sequences, 2,000 rules with 16 element pairs each: **219.0 -> 174.8 ms** (**20.2% reduction**). ### End-to-end cold compile Measured with the GLM-5.2 tokenizer (155,000-token vocabulary), `max_threads=8`, and a fresh compiler for each run. Values are pooled medians over 10 cold compiles: - Flat JSON Schema with 50,000 fields, elapsed wall time: **4440.5 -> 4237.0 ms** (**4.6% reduction**). - Total process CPU time, using the median of the two five-run medians: **10478.8 -> 10056.6 ms** (**4.0% reduction**). The end-to-end gain is smaller because tokenizer-vocabulary compilation and other stages are unchanged. Mask generation is unaffected: this PR changes compile-time FSM construction, and the resulting FSMs are byte-identical.
改动
本请求降低首次语法编译在结构化标签分派、JSON Schema(用于描述 JSON 数据结构的规格)转换、语法优化和有限状态机构造中的开销。有限状态机是用状态和转换表示允许路径的数据结构。
原因
大型规格和大型结构化标签集会同时触发多种首次启动开销:重复子串匹配、状态与词元之间的密集计算、大型语法文本生成、无效优化重写和状态机复制。本改动分别去除这些开销,不改变原有语法与匹配行为。
性能
验证
d6fc0420。