Bound tree-sitter parse memory with a wall-clock budget (APP-4667) - #15150
Bound tree-sitter parse memory with a wall-clock budget (APP-4667)#15150warp-agent-staging[bot] wants to merge 2 commits into
Conversation
Tree-sitter's error-recovery pass (ts_parser__recover) can allocate memory super-linearly on dense-error inputs regardless of file size. The existing MAX_PARSE_BYTES cap (2 MiB) does not prevent this: field reports show a sub-cap buffer driving ~9.29 GB of tree-sitter parsing/error-recovery memory. Add PARSE_BUDGET, a 750ms wall-clock deadline enforced via tree-sitter's progress callback (parse_with_options), so a pathological parse bails out to the existing "no syntax tree" fallback instead of running unbounded. The callback is polled roughly every 100 parse actions by tree-sitter itself, bounding a runaway parse's overshoot to a small, input-size- independent amount of extra work rather than growing with N. To avoid re-spending the budget on every keystroke once a buffer proves pathological, latch parsing off for that buffer (SyntaxTreeState) after a budget trip; the latch clears when the language is reset (e.g. reopening the file). Log a single warn! breadcrumb per trip for field observability. Keep MAX_PARSE_BYTES as a cheap, size-based first line of defense (a size comparison can never itself run away), but it no longer carries the whole guarantee. Add regression tests: a dense-error SQL parse under MAX_PARSE_BYTES trips PARSE_BUDGET instead of completing, an oversized buffer still short- circuits on size alone, and a latched buffer skips re-parsing entirely on the next edit. CHANGELOG-BUG-FIX: Fixed a memory spike where editing files with dense syntax errors could cause tree-sitter's error recovery to consume multiple gigabytes of memory; such buffers now fall back to no syntax highlighting instead. Co-Authored-By: Warp <agent@warp.dev>
warp-agent-staging
Bot
added
area:performance:memory
|
This PR was generated with Warp.   |
![warp-agent-staging[bot]](https://avatars.githubusercontent.com/in/2196476?s=60&v=4){kind=small}
There was a problem hiding this comment.
Overview
Bounds tree-sitter parsing with a 750ms wall-clock deadline enforced through parse_with_options' progress callback, falling back to the existing no-syntax-tree path. Two design questions below need your decision; separately identified correctness defects are being revised and will follow as commits.
Verdict
Checks: build pass, tests pass (10/10 syntax_tree), CI green (5 passed, 15 skipped), visual proof missing
Found: 0 critical, 0 important, 0 suggestions, 0 nits, 2 questions — both awaiting your judgment, not a request for changes.
Responding as wilson: Open session · View factory task
| /// input took ~700ms/~220MB unbounded; an ~8.5MB one took ~3s/~870MB). 750ms | ||
| /// comfortably covers legitimate full parses of files up to the size cap while | ||
| /// keeping the worst-case bailout in the low seconds instead of unbounded. | ||
| const PARSE_BUDGET: Duration = Duration::from_millis(750); |
There was a problem hiding this comment.
question — a wall-clock deadline is not a memory bound, and the bug is a memory blowup.
In the pinned arborium-tree-sitter 2.13.0, the progress callback is only checked once the operation counter reaches 100 (src/parser.c), and a single operation can enter ts_parser__recover and do input-size-dependent stack/tree work before returning to that check. So the measured ~2x overshoot is not guaranteed to hold: a machine faster than the M1 Pro in the report allocates more within the same 750ms, and nothing here imposes an allocation ceiling.
Decision needed: is a latency proxy an acceptable bound for this, or do you want a real per-parse memory ceiling (allocator/resource limit, or a structural content guard) with this deadline kept only as a secondary latency limit?
Responding as wilson: Open session · View factory task
| syntax_query: HighlightQuery::new(&language.highlight_query, self.color_map), | ||
| language, | ||
| }); | ||
| self.parse_budget_exceeded = false; |
There was a problem hiding this comment.
question — the latch reset fires on theme changes but not on content fixes.
Because set_color_map calls set_language, an unrelated appearance change silently re-arms tree-sitter on a buffer already known to be pathological, while correcting the offending content does not restore highlighting until the file is reopened. Combined with the deadline being load-sensitive, a transient 750ms miss (loaded machine, resume from sleep, debug build) can permanently degrade a benign buffer.
Decision needed: may a transient miss permanently disable highlighting, and what should reset it? If the intent is a language/new-buffer transition, the latch should survive color-map-only updates.
Responding as wilson: Open session · View factory task
…n the parse budget Address review findings on the initial PARSE_BUDGET implementation: 1. Aborting the previous ctx.spawn task did not actually interrupt an in-flight blocking tree-sitter parse (Abortable only checks between .await points, and the blocking parse has none). Rapid edits could pile up multiple full-budget parses on the background executor, multiplying the exact memory exposure this feature exists to bound. Fixed by threading a shared cancellation flag into the parse's own progress callback, and serializing/coalescing: only one parse is ever in flight per buffer; a newer edit signals the current one to bail out and is dispatched once that completion is observed. 2. Every `None` from parse_with_options was treated as BudgetExceeded, even though tree-sitter can also return None for unrelated reasons (e.g. a scanner error). Fixed by tracking whether *our* callback actually triggered the cancellation (and why) via a new CancelReason, classified by a pure, unit-testable `classify_parse_result` function. Added a distinct `ParseOutcome::Failed` for this case, and a `Superseded` outcome for cancellations caused by coalescing (neither trips the latch or logs a false timeout). 3. Parse completions were applied with no generation check, so a stale result could in principle latch/discard a newer, unrelated buffer version. Added a generation counter; a completion is only applied if it still matches the state's current generation. 4. Rewrote the test suite to remove all wall-clock-dependent assertions. `SyntaxTreeState` now carries a `parse_budget: Duration` field (test-only overridable) and `parse_text` takes an explicit `deadline` and `cancel` flag, so tests can deterministically drive real BudgetExceeded/Superseded completions through the actual production code path and assert on the real side effects (latch set, tree discarded, DecorationUpdated emitted, no replacement parse scheduled) instead of asserting on elapsed time or poking private state. Co-Authored-By: Warp <agent@warp.dev>
There was a problem hiding this comment.
Overview
Replaces the insufficient 2 MiB size cap as the sole guard with a 750 ms parse deadline enforced through tree-sitter's progress callback, falling back to the existing no-syntax-tree path. Four correctness defects found in review were fixed in b9570cd; the two design questions below need a human decision and are the reason this is not a straight approval.
Concerns
- The fallback has no in-UI notice. Visual verification on macOS confirmed the buffer silently renders unhighlighted; the only status text present, "Language support is unavailable for this file type", also shows for files that are highlighted, so it is unrelated. A user whose highlighting vanishes has nothing to tell them why — decide whether that needs a surface before this ships.
- Verification could not demonstrate mouse-wheel scrolling in the unhighlighted buffer; scrollbar dragging worked and was used instead. Whether that is an automation artifact or real editor behavior is undetermined and untested either way.
Verdict
Checks: build pass, tests pass (13/13 in syntax_tree, deterministic), CI green — though the Formatting+Clippy and test jobs report skipping on this draft rather than having actually run — visual proof present (macOS 26.3.1 arm64, recording and stills in the description)
Found: 0 critical, 0 important outstanding (4 important raised in review and fixed in b9570cd), 0 suggestions, 0 nits, 2 questions
Responding as wilson: Open session · View factory task
Description
Fixes APP-4667: tree-sitter's error-recovery pass (
ts_parser__recover) can allocate memory super-linearly on dense-error inputs regardless of file size. Field reports (Sentry) show a buffer well under the existingMAX_PARSE_BYTEScap (2 MiB) driving ~9.29 GB of tree-sitter parsing/error-recovery memory — the size cap alone is provably insufficient.What's new: a
PARSE_BUDGET(750ms wall-clock) enforced via tree-sitter'sparse_with_optionsprogress callback. When a parse exceeds the budget, it's cancelled and the buffer falls back to the existing "no syntax tree" path (same as exceedingMAX_PARSE_BYTEStoday).What a user loses when it trips: syntax highlighting for that specific buffer — the editor still renders and functions normally, just without coloring. Once a buffer trips the budget, further tree-sitter parsing for it is skipped entirely (latched) until its language is reset (e.g. the file is reopened), rather than re-attempting a full parse on each edit.
MAX_PARSE_BYTES(2 MiB) is kept as a cheap first line of defense — it's a size comparison, not a parse attempt, so it can never itself run away — but it no longer carries the whole guarantee;PARSE_BUDGETis the actual bound on the parse itself.Design notes
true= cancel (easy to get backwards; see tree-sitter#4312 — an older draft attempt at this had it inverted).OP_COUNT_PER_PARSER_TIMEOUT_CHECKin its ownparser.c), not on a fixed time or byte cadence. That bounds a runaway parse's overshoot past the deadline to "however long one more ~100-action batch takes" rather than something that scales with N. In local benchmarking that overshoot was within roughly 2x the budget even for inputs at theMAX_PARSE_BYTEScap, but because per-action cost itself grows under the pathological workload this targets, it isn't a hard guarantee — it's a large practical improvement over unbounded growth, not a mathematically tight bound. (Flagged as an open question for the requester below.)abort().ModelContext::spawn'sAbortHandle/Abortableonly observes cancellation when the wrapped future is polled at an.awaitpoint — the blocking tree-sitter call has none until it returns, so callingabort()on a superseded task does not stop the in-flight parse from continuing to allocate. Fixed by threading anArc<AtomicBool>cancellation flag into the parse's own progress callback (checked alongside the deadline), and serializing: only one parse is ever in flight per buffer — a new edit while a parse is running signals it to cancel and coalesces itself into apending_edit, dispatched once that parse's completion is observed. This closes what was otherwise a per-parse (not per-buffer) bound: rapid edits on a pathological buffer could previously pile up multiple full-budget parses on the background executor.parse_with_optionscan returnNonefor reasons other than our own cancellation (e.g. a scanner error). ACancelReason(set only when our callback decides to cancel, and why) distinguishes a confirmed deadline trip (BudgetExceeded, which latches) from a coalescing cancellation (Superseded, silently discarded — not a pathological buffer, just normal fast typing) from an unrelated parser failure (Failed, falls back the same way but does not latch or log a false timeout). Classification lives in a small pure function (classify_parse_result) for direct unit testing.Out of scope
GlobalBufferModel::resolve_conflict -> Buffer::replace_all -> populate_buffer_with_read_contentmemory site (app/src/code/global_buffer_model.rs, ~9.72 GB in the same profile) is a different mechanism — replacing a buffer's content re-materializes it into a newSumTree<BufferText>while the old tree and an intermediate formatted-text representation are still live. This looks like the expected (if non-trivial, ~2-4x) cost of a full-buffer replace rather than an unbounded/pathological blowup, but I did not fully traceapply_core_edit_actions/convert_text_with_style_to_formatted_text's peak-memory behavior or have the actual file size from the Sentry incident, so I'm not fully confident. Flagging as a candidate follow-up rather than folding a speculative fix into this PR.Open questions for the requester (not resolved in this PR)
set_color_mapcallsset_language, so a theme change silently re-arms a previously-latched pathological buffer, while fixing the buffer's content does not restore highlighting on its own (see inline PR comment).Linked Issue
Testing
crates/syntax_tree/src/lib_tests.rs(13 tests total, all deterministic, no wall-clock-dependent assertions):test_classify_parse_result_distinguishes_none_reasons: pure-function coverage of the deadline vs. superseded vs. failed classification.test_parse_exceeding_deadline_falls_back_instead_of_completing: a dense-error SQL input underMAX_PARSE_BYTES, with an already-elapsed deadline, returnsBudgetExceededinstead of completing.test_parse_returns_superseded_when_cancel_flag_is_set: a pre-armed cancellation flag (deadline far in the future) returnsSuperseded, notBudgetExceeded.test_parse_skips_oversized_buffer_without_attempting_to_parse: buffers overMAX_PARSE_BYTESstill short-circuit on the cheap size check.test_budget_exceeded_completion_latches_discards_tree_and_stops_reparsing: drives a realBudgetExceededcompletion throughupdate_internal_state_with_delta(via a test-only overridable parse budget) and asserts on the actual production side effects — latch set, tree removed from the map, no parse left in flight,DecorationUpdatedemitted (observed via a real model-event subscription), and a further edit does not re-dispatch a parse.test_rapid_edits_coalesce_instead_of_running_concurrent_parses: two edits arriving before the first parse's completion is observed result in the cancel flag being signaled and the second edit being coalesced intopending_edit, rather than a second concurrent parse starting.cargo test -p syntax_tree— 13/13 pass in 0.42s.cargo clippy -p syntax_tree --all-targets --all-features --tests -- -D warnings— clean../script/format— no changes needed.cargo check -p aiandcargo check -p warp --bin warp(downstream consumers ofsyntax_tree) — both compile cleanly; no public API ofSyntaxTreeStatechanged.Standalone benchmark (pinned to the exact
arborium/arborium-tree-sitter2.13.0 used by this repo'sCargo.lock, not committed) against dense-error SQL, unbounded:I could not reproduce the full ~9.29 GB from the field report locally (my synthetic pattern tops out in the hundreds of MB before hitting the byte cap), but this does reproduce the same class of bug: a buffer safely under the current size cap driving 100-400x its own size in memory via tree-sitter parsing.
Visual proof: captured on macOS (recording and screenshots above). macOS 26.3.1 arm64, 8 cores / 16 GB; debug build of this branch at
b9570cd, bundled and launched as a real.appvia the repo's documented./script/runpath, logged out..sqlfile renders with full syntax highlighting — keywords, types/constraints, string and numeric literals, and function names each distinctly colored..sqlfile (52,326 lines of unclosed nesting, ~575k unclosed parens by EOF) — 4,112 bytes under the 2 MiBMAX_PARSE_BYTEScap, so it takes the parse path rather than the size-skip path — renders as uniform white with no color differentiation anywhere, checked at the top and around lines 16k, 33k and 44k.SELECT 1;at line 33020 inserted inline immediately and the unsaved-changes indicator updated. No hang, beachball or crash at any point.~/Library/Logs/warp_local.log, not inferred from the rendering — exactly one occurrence of:[WARN] [syntax_tree] [SyntaxTreeState] tree-sitter parse exceeded 750ms budget; disabling syntax highlighting for this buffer until its language is resetNot verified: whether this fix eliminates the exact Sentry alert in production — that needs field observation post-deploy (see the
warn!log added for this).Computer-use video recordings
View video recording - Warp's built-in editor on macOS: a small valid SQL file renders with syntax highlighting, then a 2,093,040-byte dense-error SQL file (just under the 2 MiB cap) renders unhighlighted but stays fully scrollable and editable as the 750 ms parse budget trips.
Computer-use screenshots
small_valid.sql— syntax highlighting presenthuge_malformed.sql— content rendered, uniform white, no highlightingAgent Mode
CHANGELOG-BUG-FIX: Fixed a memory spike where editing files with dense syntax errors could cause tree-sitter's error recovery to consume multiple gigabytes of memory; such buffers now fall back to no syntax highlighting instead.