Reduce no-tree parser overhead - #115
Conversation
WalkthroughThe fast recognizer now reuses top-level visiting and memo buffers, sizes memo storage from buffered token counts, and releases oversized memo allocations. Parser reset aligns token-node storage with parse-tree configuration. Rule transitions and final outcome handling skip deferred-node construction and materialization when parse trees are disabled. Tests cover memo capacity behavior, scratch reuse and release, and empty recognition-arena node storage. Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Copy/Paste DetectionFound 16 duplication(s) across 1 changed Rust file(s) (threshold: 100 tokens). Show duplicationsFound a 27 line (145 tokens) duplication in the following files:
fn generated_match_token_recovers_missing_token_from_context_follow() {
let atn = generated_match_recovery_atn();
let data = RecognizerData::new(
"Mini.g4",
Vocabulary::new(
[None, Some("'X'"), Some("'Y'")],
[None, Some("X"), Some("Y")],
[None::<&str>, None, None],
),
);
let mut parser = BaseParser::new(
CommonTokenStream::new(Source {
tokens: vec![TestToken::eof("parser-test", 3, 1, 3)],
index: 0,
}),
data,
);
parser.rule_context_stack = vec![
RuleContextFrame {
rule_index: 0,
invoking_state: 0,
},
RuleContextFrame {
rule_index: 1,
invoking_state: 1,
},
];Found a 27 line (127 tokens) duplication in the following files:
let mut atn = ParserAtnBuilder::new(2);
assert_eq!(
atn.add_state(AtnStateKind::RuleStart, Some(0))
.expect("state")
.index(),
0
);
assert_eq!(
atn.add_state(AtnStateKind::BlockStart, Some(0))
.expect("state")
.index(),
1
);
assert_eq!(
atn.add_state(AtnStateKind::Basic, Some(0))
.expect("state")
.index(),
2
);
assert_eq!(
atn.add_state(AtnStateKind::Basic, Some(0))
.expect("state")
.index(),
3
);
assert_eq!(
atn.add_state(AtnStateKind::BlockEnd, Some(0))Found a 33 line (126 tokens) duplication in the following files:
if self.fast_parser_predicate_matches(predicate_context, transition, index) {
let boundary = left_recursive_boundary(atn, state, target);
outcomes.extend(
self.recognize_state_fast(
atn,
FastRecognizeRequest {
state_number: target,
stop_state,
index,
rule_start_index,
decision_start_index: next_decision_start_index,
precedence,
depth: depth + 1,
recovery_symbols: Rc::clone(&epsilon_recovery_symbols),
recovery_state: epsilon_recovery_state,
},
FastRecognizeScratch {
predicate_context,
visiting,
memo,
expected,
},
)
.into_iter()
.map(|mut outcome| {
if let Some(rule_index) = boundary {
let boundary = self.arena_boundary_node(rule_index);
self.defer_fast_outcome_node(&mut outcome, boundary);
}
outcome
}),
);
} else {Found a 32 line (125 tokens) duplication in the following files:
let boundary = left_recursive_boundary(atn, state, target);
outcomes.extend(
self.recognize_state_fast(
atn,
FastRecognizeRequest {
state_number: target,
stop_state,
index,
rule_start_index,
decision_start_index: next_decision_start_index,
precedence,
depth: depth + 1,
recovery_symbols: Rc::clone(&epsilon_recovery_symbols),
recovery_state: epsilon_recovery_state,
},
FastRecognizeScratch {
predicate_context,
visiting,
memo,
expected,
},
)
.into_iter()
.map(|mut outcome| {
if let Some(rule_index) = boundary {
let boundary = self.arena_boundary_node(rule_index);
self.defer_fast_outcome_node(&mut outcome, boundary);
}
outcome
}),
);
}Found a 22 line (125 tokens) duplication in the following files:
atn.add_state(AtnStateKind::BlockEnd, Some(0))
.expect("state")
.index(),
4
);
assert_eq!(
atn.add_state(AtnStateKind::RuleStop, Some(0))
.expect("state")
.index(),
5
);
atn.set_rule_to_start_state(vec![0])
.expect("rule start states");
atn.set_rule_to_stop_state(vec![5])
.expect("rule stop states");
atn.add_decision_state(1).expect("decision state");
atn.add_transition(0, ParserTransitionSpec::Epsilon { target: 1 })
.expect("transition");
atn.add_transition(
1,
ParserTransitionSpec::Atom {
target: 2,Found a 33 line (115 tokens) duplication in the following files:
outcomes.extend(
self.recognize_state(
atn,
RecognizeRequest {
state_number: *target,
stop_state,
index,
rule_start_index,
decision_start_index: next_decision_start_index,
init_action_rules,
predicates,
semantics,
rule_args,
member_actions,
return_actions,
local_int_arg,
member_values: member_values.clone(),
return_values: return_values.clone(),
rule_alt_number: next_alt_number,
track_alt_numbers,
consumed_eof,
precedence,
depth: depth + 1,
recovery_symbols: epsilon_recovery_symbols.clone(),
recovery_state: epsilon_recovery_state,
},
visiting,
memo,
expected,
)
.into_iter()
.map(|mut outcome| {
prepend_decision(&mut outcome, decision);Found a 15 line (113 tokens) duplication in the following files:
fn generated_match_token_counts_single_token_deletion_recovery() {
let atn = generated_match_recovery_atn();
let data = RecognizerData::new(
"Mini.g4",
Vocabulary::new(
[None, Some("'X'"), Some("'Y'"), Some("'Z'")],
[None, Some("X"), Some("Y"), Some("Z")],
[None::<&str>, None, None, None],
),
);
let mut parser = BaseParser::new(
CommonTokenStream::new(Source {
tokens: vec![
TestToken::new(3).with_text("z"),
TestToken::new(2).with_text("y"),Found a 12 line (112 tokens) duplication in the following files:
let mut atn = ParserAtnBuilder::new(1);
for (state, kind, rule) in [
(0, AtnStateKind::RuleStart, 0),
(1, AtnStateKind::StarLoopEntry, 0),
(2, AtnStateKind::Basic, 0), // ops hub
(3, AtnStateKind::Basic, 0), // shift prec
(4, AtnStateKind::Basic, 0), // shift first >
(5, AtnStateKind::Basic, 0), // shift second >
(6, AtnStateKind::Basic, 0), // rel prec
(7, AtnStateKind::Basic, 0), // rel >
(8, AtnStateKind::LoopEnd, 0),
(9, AtnStateKind::RuleStop, 0),Found a 22 line (112 tokens) duplication in the following files:
fn predicate_after_token_atn() -> Atn {
let mut atn = ParserAtnBuilder::new(2);
assert_eq!(
atn.add_state(AtnStateKind::RuleStart, Some(0))
.expect("state")
.index(),
0
);
assert_eq!(
atn.add_state(AtnStateKind::Basic, Some(0))
.expect("state")
.index(),
1
);
assert_eq!(
atn.add_state(AtnStateKind::Basic, Some(0))
.expect("state")
.index(),
2
);
assert_eq!(
atn.add_state(AtnStateKind::Basic, Some(0))Found a 22 line (111 tokens) duplication in the following files:
fn left_recursive_loop_with_caller_follow_atn(caller_symbol: i32) -> Atn {
let mut atn = ParserAtnBuilder::new(2);
assert_eq!(
atn.add_state(AtnStateKind::RuleStart, Some(0))
.expect("state")
.index(),
0
);
assert_eq!(
atn.add_state(AtnStateKind::Basic, Some(0))
.expect("state")
.index(),
1
);
assert_eq!(
atn.add_state(AtnStateKind::Basic, Some(0))
.expect("state")
.index(),
2
);
assert_eq!(
atn.add_state(AtnStateKind::RuleStart, Some(1))Found a 14 line (110 tokens) duplication in the following files:
fn parser_matches_token_and_reports_mismatch() {
let source = Source {
tokens: vec![
TestToken::new(1).with_text("x"),
TestToken::eof("parser-test", 1, 1, 1),
],
index: 0,
};
let data = RecognizerData::new(
"Mini.g4",
Vocabulary::new([None, Some("'x'")], [None, Some("X")], [None::<&str>, None]),
);
let mut parser = BaseParser::new(CommonTokenStream::new(source), data);
let matched = parser.match_token(1).expect("token 1 should match");Found a 13 line (109 tokens) duplication in the following files:
fn parser_matches_token_and_reports_mismatch() {
let source = Source {
tokens: vec![
TestToken::new(1).with_text("x"),
TestToken::eof("parser-test", 1, 1, 1),
],
index: 0,
};
let data = RecognizerData::new(
"Mini.g4",
Vocabulary::new([None, Some("'x'")], [None, Some("X")], [None::<&str>, None]),
);
let mut parser = BaseParser::new(CommonTokenStream::new(source), data);Found a 22 line (108 tokens) duplication in the following files:
) -> Option<RecognizeOutcome> {
let (error_index, message) = self.expected_error_message(rule_index, start_index, expected);
let diagnostic = diagnostic_for_token(self.token_at(error_index), message);
let mut next_index = error_index;
loop {
let symbol = self.token_type_at(next_index);
if sync_symbols.contains(&symbol) {
if next_index == error_index {
return None;
}
break;
}
if symbol == TOKEN_EOF {
break;
}
let after = self.consume_index(next_index, symbol);
if after == next_index {
break;
}
next_index = after;
}
let mut nodes = NodeSeqId::EMPTY;Found a 15 line (108 tokens) duplication in the following files:
fn outcome_ties_keep_later_non_recursive_alternative() {
let arena = RecognitionArena::default();
let first = RecognizeOutcome {
index: 1,
consumed_eof: false,
alt_number: 0,
member_values: BTreeMap::new(),
return_values: BTreeMap::new(),
diagnostics: DiagnosticSeqId::EMPTY,
decisions: Vec::new(),
actions: vec![ParserAction::new(1, 0, 0, None)],
nodes: NodeSeqId::EMPTY,
};
let second = RecognizeOutcome {
actions: vec![ParserAction::new(2, 0, 0, None)],Found a 16 line (105 tokens) duplication in the following files:
let start_state = atn.rule_to_start_state().get(rule_index).ok_or_else(|| {
AntlrError::Unsupported(format!("rule {rule_index} has no start state"))
})?;
let stop_state = atn
.rule_to_stop_state()
.get(rule_index)
.filter(|state| *state != usize::MAX)
.ok_or_else(|| {
AntlrError::Unsupported(format!("rule {rule_index} has no stop state"))
})?;
let start_index = self.current_visible_index();
self.clear_prediction_diagnostics();
self.reset_per_parse_caches();
self.reset_recognition_arena();
let caller_follow_state = self.pending_invoking_follow_state(atn);Found a 13 line (100 tokens) duplication in the following files:
let mut expected = BTreeSet::new();
for index in (1..self.rule_context_stack.len()).rev() {
let invoking_state = self.rule_context_stack[index].invoking_state;
let Ok(state_number) = usize::try_from(invoking_state) else {
continue;
};
let Some(Transition::Rule { follow_state, .. }) = atn
.state(state_number)
.and_then(|state| state.transitions().first())
.map(ParserTransition::data)
else {
continue;
}; |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request optimizes the parser by reusing cycle and memo storage for fast recognition via FastRecognizeTopScratch and skipping node storage when parse tree building is disabled. A critical issue was identified in the prepare method of FastRecognizeTopScratch, where using saturating_sub with reserve after clear() fails to guarantee the requested capacity, potentially leading to unnecessary reallocations. It is recommended to call reserve directly with the target capacity.
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.
tinovyatkin
changed the title
|
@codex review |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/parser.rs (1)
6349-6350: 🚀 Performance & Scalability | 🟠 Major | ⚡ Quick winRestore
fast_token_nodes_enabledtoself.build_parse_treesinstead of unconditionallytrue.This unconditionally enables token-node storage for the retry passes, even when
build_parse_treesis disabled. This leaks the token-node overhead back into the retry path and overrides the synchronization done inreset().To preserve the no-tree optimization during retries, set this to
self.build_parse_trees.⚡ Proposed fix to preserve the optimization on retries
let first_pass = self.fast_recognize_top(atn, top_request, predicate_context); - self.fast_token_nodes_enabled = true; + self.fast_token_nodes_enabled = self.build_parse_trees;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/parser.rs` around lines 6349 - 6350, Update the assignment following fast_recognize_top in the relevant parser retry flow to set self.fast_token_nodes_enabled from self.build_parse_trees rather than unconditionally enabling it, preserving the no-tree optimization and reset synchronization.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/parser.rs`:
- Around line 15087-15093: Add a second prepare call in
fast_recognize_scratch_reuses_small_tables_and_releases_oversized_memo using a
larger capacity after the initial preparation, then assert that memo.capacity()
reaches at least the requested size. Keep the existing retention assertions so
the test covers expansion from a non-zero capacity and the oversized-memo
release behavior.
- Around line 1286-1297: Update prepare so the reserve calls use the full target
capacities after clear: reserve FAST_RECOGNIZE_VISITING_CAPACITY for
self.visiting and reserve memo_capacity for self.memo, while retaining the
existing capacity checks and avoiding subtraction of current capacity.
---
Outside diff comments:
In `@src/parser.rs`:
- Around line 6349-6350: Update the assignment following fast_recognize_top in
the relevant parser retry flow to set self.fast_token_nodes_enabled from
self.build_parse_trees rather than unconditionally enabling it, preserving the
no-tree optimization and reset synchronization.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d2812e46eb
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
|
Codex Review: Didn't find any major issues. Breezy! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
1 participant
Summary
Root cause
The fast recognizer allocated fresh cycle-detection and memo hash tables for every top-level parse and reserved at least 65,536 memo entries even for small statements. The no-tree path used by the MySQL benchmark also continued constructing and materializing deferred rule nodes that the caller could never observe, and retry passes re-enabled token-node storage unconditionally.
This keeps the runtime and codegen grammar-agnostic while removing those sources of avoidable parser work.
Performance
Same-machine Apple M3 Pro, alternating current-main/head/head/current-main. Each binary ran the benchmark's 1 cold + 5 warm iterations, dropped the two slowest warm parse runs, and averaged the remaining three.
mainparse avgstatements.txtbitrix_queries_cut.sqlsakila-data.sqlThe repository's protected parse-benchmark comparator also passed all 12 Rust fixture pairs at the 1.15x regression threshold, including the Kotlin Rust-over-Go 3.0x requirement.
Progress on #113.
Validation
cargo test --locked --all-targets --all-featurescargo clippy --locked --all-targets --all-features -- -D warningscargo run --release --quiet --bin antlr4-runtime-testsuite(357 passed, 0 failed, 0 skipped)python3 tools/parse-bench/compare.py --baseline ... --current ... --max-regression 1.15 --require-speedup kotlin:rust-antlr:go-antlr:3.0