fix(editor): stop large SQL paste from freezing the editor#1654
Conversation
![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: 6ba5040444
ℹ️ 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".
| let execAsync = Self.shouldExecuteAsync( | ||
| editLength: range.length, | ||
| delta: delta, | ||
| documentLength: textView.documentRange.length | ||
| ) |
There was a problem hiding this comment.
When this new gate returns true for a large caret paste, applyEdit now takes the async executor path; that path still invokes the @MainActor completion directly from TreeSitterExecutor's background Task (operation: { completion(.success(operation())) }). In the inspected highlighter flow (Highlighter.textStorage(...didProcessEditing...) → HighlightProviderState.storageDidUpdate), that completion mutates highlight invalidation state and can touch UI-facing state off the main actor for exactly the large paste case this commit adds to the async path. The async branch should hop back to the main actor/queue before calling the completion, as the query and cancel paths already do.
Useful? React with 👍 / 👎.
datlechin
force-pushed
the
worktree-fix-1652-large-sql-paste
branch
from
6ba5040 to
f6a67d5
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b634164318
ℹ️ 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".
| ) | ||
| highlighter.maxHighlightableLength = 1_000 | ||
|
|
||
| textView.replaceCharacters(in: NSRange(location: 0, length: 0), with: "x") |
There was a problem hiding this comment.
In this new test the TextView never adds highlighter as a storage delegate, so replaceCharacters does not call Highlighter.textStorage(_:didProcessEditing:...) and queryCount remains at its setup value. The nearby edit integration test registers with textView.addStorageDelegate(highlighter) before editing; this test should do the same or it fails while not actually covering the under-limit edit path.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 58e0d5a08d
ℹ️ 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".
| if !isLargeDocument { | ||
| self?.inlineSuggestionManager?.handleTextChange() | ||
| } |
There was a problem hiding this comment.
When a small edit has already scheduled a debounced inline suggestion, a subsequent paste that pushes the document over languageServiceLengthLimit takes this branch and never calls handleTextChange(), so the existing debounceTask/requestTask is not cancelled. When that task fires, InlineSuggestionManager.requestSuggestion() still reads textView.string into fullText and the AI inline source can send the whole large query, which reintroduces the large-document copy/work this gate is meant to avoid; the large-document path should still dismiss/cancel pending suggestions rather than doing nothing.
Useful? React with 👍 / 👎.
| /// Documents longer than this are not highlighted. Above this length the per-edit re-parse and re-highlight cost | ||
| /// dominates, so the editor stops syntax highlighting to keep typing, scrolling, and deleting responsive, the same | ||
| /// way DataGrip and VS Code degrade large files. | ||
| public var maxHighlightableLength: Int = 2_000_000 |
There was a problem hiding this comment.
Lowering this cutoff makes 2–5 MB documents skip Highlighter.textStorage(_:didProcessEditing:), whose guard returns before styleContainer.storageUpdated and highlightProviders.storageDidUpdate. If the user edits while the document is above 2 MB and then deletes enough text to fall back under the limit, the next processed edit is applied to stale style/tree-sitter state that missed the over-limit edits, so highlighting can be incorrect or fail until the editor is recreated; crossing back under the cap should rebuild/reset provider state instead of applying only the final edit.
Useful? React with 👍 / 👎.
datlechin
force-pushed
the
worktree-fix-1652-large-sql-paste
branch
from
58e0d5a to
03a7f5b
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 03a7f5bdf0
ℹ️ 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".
| range editedRange: NSRange, | ||
| changeInLength delta: Int | ||
| ) { | ||
| guard processesEdits else { return } |
There was a problem hiding this comment.
When a hidden minimap sets processesEdits = false, returning here also skips attachments.textUpdated(atOffset:delta:). If the document has a text attachment such as a folded range and the user edits before it while the minimap is hidden, the main layout manager shifts/removes that attachment but the minimap's mirrored attachment ranges stay stale; reset() rebuilds only line storage, not attachments, so showing the minimap again can lay out folds at the wrong offsets. The suspended path should still keep attachment ranges in sync or rebuild/clear them on reset.
Useful? React with 👍 / 👎.
…nter not the whole string
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 82ae24639b
ℹ️ 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".
| /// Documents longer than this are not highlighted. Above this length the per-edit re-parse and re-highlight cost | ||
| /// dominates, so the editor stops syntax highlighting to keep typing, scrolling, and deleting responsive, the same | ||
| /// way DataGrip and VS Code degrade large files. | ||
| public var maxHighlightableLength: Int = 2_000_000 |
There was a problem hiding this comment.
When a script is opened or TextViewController.setText replaces the storage, setUpHighlighter() still calls highlighter.invalidate(), and Highlighter.invalidate() forwards directly to each provider without checking this cutoff. That means documents over the new 2 MB limit still run the initial tree-sitter setup/query path even though the edit and visible-range callbacks now return early, so opening or programmatically loading a large SQL script can still do the syntax-highlighting work this limit is meant to suspend. The cutoff needs to be enforced on invalidation/setup as well, not just on edits.
Useful? React with 👍 / 👎.
…flatten query equality
1 participant
Fixes #1652.
Problem
Pasting a large block of SQL froze the editor; restart required. Profiling with temporary OSLog timing then showed two further problems on a ~50k-line / ~4 MB document: typing pinned the CPU, and paste / select-all-delete each took ~500-600 ms.
Fixes (three commits)
1. Paste no longer freezes (
fix(editor))The one-shot insertion runs synchronously inside
NSTextStorage.endEditing(), a path earlier large-document work (#701) never covered.TextLayoutManager+Edits.swift: the line-index rebuild allocated a bridgedStringper line; rewritten to pass length + a line-break flag (behavior-preserving).TreeSitterClient.swift: the async gate now keys offmax(replaced, abs(delta)), so a big caret paste parses off the main thread.2. Large documents stop pinning the CPU (
perf(editor))A 3 MB document sat under the old 5 MB highlight cap, so every keystroke re-parsed and re-highlighted, and inline AI copied the whole document per keystroke.
Highlighter.swift:maxHighlightableLength5 MB → 2 MB. The guard precedes both the highlight and the tree-sitter parse, so above 2 MB the editor stops both and stays responsive, the way DataGrip (2.5 MB) and VS Code degrade large files.SQLEditorCoordinator.swift: above the same cutoff, skip the inline-AI document sync.3. Paste / delete-all run the line rebuild once, not twice (
perf(editor))Timing showed both were dominated by the line-storage rebuild running twice. A hidden minimap (
showMinimap: false) still created aTextLayoutManagerthat rebuilt the line index on every edit.MinimapViewsuspends its layout manager (processesEdits = false) while hidden and rebuilds it on show. The hidden minimap no longer does a second full rebuild on every paste, delete, or keystroke.4. Typing in a large query no longer pins the CPU (
perf(editor))A Time Profiler trace showed the main thread spending its time in SwiftUI's attribute-graph diff (
AG::LayoutDescriptor::Compare->_stringCompareSlow->Unicode._NFCNormalizer): on every view-graph update, SwiftUI compared the whole ~4 MB query for equality with canonical Unicode normalization, becauseTabQueryContentis anEquatablevalue type whose synthesized==comparesquery: String. The query is bridged from the editor'sNSTextStorage, so it took the slow foreign-string path.QueryTabState.swift:TabQueryContentnow has a custom==that compares the text withNSStringliteral equality, which returns in O(1) when the lengths differ (every keystroke) and skips NFC normalization. Literal comparison is also the correct semantics for SQL text.5. Hidden editor peripherals stop doing whole-document work per keystroke (
perf(editor))A second Time Profiler trace (after fix 4) still showed ~40% of the main thread in canonical Unicode comparison and
CFStorageGetConstValueAtIndexwhile typing. Two more sources, both in the vendored editor:TextBindingSync.applyRepresentableTextcompared the multi-megabyte bridged text with Swift!=(canonical NFC). Now usesNSStringliteral equality.showFoldingRibbon: false.LineFoldModelnow skips the recompute while the ribbon is hidden and rebuilds when it is shown (same pattern as the hidden minimap in fix 3).6. SwiftUI stops normalizing the whole query on every diff (
perf(editor))A third trace showed the main thread still ~40% in
AGGraphSetOutputValue -> AG::LayoutDescriptor::Compare -> String._stringCompareSlow(NFC). SwiftUI's attribute graph compares a view's body output by walking its memory layout, so it reached the rawquery: Stringnested inQueryTaband normalized the full 4 MB on every update. A customEquatabledoes not help here (the layout walk bypasses it).QueryTabState.swift:TabQueryContentnow stores the query behind a small immutable class (exposed via a computedvar query). The layout walk compares an 8-byte pointer instead of the string; value semantics are preserved by replacing the box on every set.Tests
TextLayoutManagerTests.largePasteMatchesFullRebuild: a large paste produces line boundaries identical to the bulk build path, across LF / CRLF / CR and trailing-break variants.TextLayoutManagerTests.suspendedLayoutManagerSkipsEditsUntilReset: a suspended layout manager ignores edits and rebuilds correctly onreset().TreeSitterClientAsyncGateTests: the async gate routes large edits async and keeps small edits sync.HighlighterTests: edits above the cap query no highlights; edits below it still do.TabQueryContentEqualityTests: equality detects length changes, same-length edits, edits in a large query, other-field changes, and that the reference-boxed query keeps value semantics.Known limitations (separate follow-ups)
Typesetter.swift, not addressed here.Verification
SwiftLint scope is
TableProonly, so the vendoredLocalPackagesedits are out of lint scope; the one in-scope file is clean. Build and tests to be run in Xcode.