Skip to content

Commit f6a67d5

Browse files
committed
fix(editor): stop large SQL paste from freezing the editor
1 parent 42ecf68 commit f6a67d5

5 files changed

Lines changed: 117 additions & 15 deletions

File tree

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
2222
- Exports no longer fail mid-table on servers that enforce a statement time limit; the export session disables the limit and restores it afterwards, the same way mysqldump does. (#1633)
2323
- Refreshing a table now reloads its data even when the previous load is still running; before, the refresh was silently dropped and the grid kept stale rows. (#1637)
2424
- SQL autocomplete now suggests tables after JOIN. It detects the clause at the cursor across multi-join and multi-clause queries, so columns no longer appear where a table is expected, and tables lead the list. (#1646)
25+
- Pasting a large SQL script no longer freezes the editor. The line index rebuild stops allocating per line, and a big paste parses off the main thread. (#1652)
2526

2627
### Security
2728

LocalPackages/CodeEditSourceEditor/Sources/CodeEditSourceEditor/TreeSitter/TreeSitterClient.swift

Lines changed: 20 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -138,6 +138,21 @@ public final class TreeSitterClient: HighlightProviding {
138138

139139
// MARK: - HighlightProviding
140140

141+
/// Decides whether an edit must be parsed asynchronously to keep the main thread responsive.
142+
///
143+
/// The magnitude of an edit is the larger of the replaced range and the inserted length. Keying only on the
144+
/// replaced range misses a large paste at the caret, where the replaced range is empty but `delta` is huge, so a
145+
/// multi-megabyte insertion into a small document would otherwise run a full re-parse synchronously.
146+
/// - Parameters:
147+
/// - editLength: The length of the range being replaced.
148+
/// - delta: The change in length, negative for deletions.
149+
/// - documentLength: The length of the document after the edit.
150+
/// - Returns: `true` when the edit should be parsed off the main thread.
151+
static func shouldExecuteAsync(editLength: Int, delta: Int, documentLength: Int) -> Bool {
152+
let editMagnitude = max(editLength, abs(delta))
153+
return editMagnitude > Constants.maxSyncEditLength || documentLength > Constants.maxSyncContentLength
154+
}
155+
141156
/// Notifies the highlighter of an edit and in exchange gets a set of indices that need to be re-highlighted.
142157
/// The returned `IndexSet` should include all indexes that need to be highlighted, including any inserted text.
143158
/// - Parameters:
@@ -161,9 +176,11 @@ public final class TreeSitterClient: HighlightProviding {
161176
return self?.applyEdit(edit: edit) ?? IndexSet()
162177
}
163178

164-
let longEdit = range.length > Constants.maxSyncEditLength
165-
let longDocument = textView.documentRange.length > Constants.maxSyncContentLength
166-
let execAsync = longEdit || longDocument
179+
let execAsync = Self.shouldExecuteAsync(
180+
editLength: range.length,
181+
delta: delta,
182+
documentLength: textView.documentRange.length
183+
)
167184

168185
if !execAsync || forceSyncOperation {
169186
let result = executor.execSync(operation)

LocalPackages/CodeEditSourceEditor/Tests/CodeEditSourceEditorTests/TreeSitterClientTests.swift

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -194,4 +194,38 @@ final class TreeSitterClientTests: XCTestCase {
194194
wait(for: editExpectations + [finalEditExpectation], timeout: 5.0)
195195
}
196196
}
197+
198+
final class TreeSitterClientAsyncGateTests: XCTestCase {
199+
private var savedEditLength = 0
200+
private var savedContentLength = 0
201+
202+
override func setUp() {
203+
savedEditLength = TreeSitterClient.Constants.maxSyncEditLength
204+
savedContentLength = TreeSitterClient.Constants.maxSyncContentLength
205+
TreeSitterClient.Constants.maxSyncEditLength = 1024
206+
TreeSitterClient.Constants.maxSyncContentLength = 1_000_000
207+
}
208+
209+
override func tearDown() {
210+
TreeSitterClient.Constants.maxSyncEditLength = savedEditLength
211+
TreeSitterClient.Constants.maxSyncContentLength = savedContentLength
212+
}
213+
214+
func test_largePasteAtCaretRunsAsync() {
215+
// The replaced range is empty for a caret paste; the inserted length must still force the async path.
216+
XCTAssertTrue(TreeSitterClient.shouldExecuteAsync(editLength: 0, delta: 500_000, documentLength: 500_000))
217+
}
218+
219+
func test_smallEditInSmallDocumentRunsSync() {
220+
XCTAssertFalse(TreeSitterClient.shouldExecuteAsync(editLength: 4, delta: 4, documentLength: 1_000))
221+
}
222+
223+
func test_largeDocumentRunsAsync() {
224+
XCTAssertTrue(TreeSitterClient.shouldExecuteAsync(editLength: 1, delta: 1, documentLength: 2_000_000))
225+
}
226+
227+
func test_largeDeletionRunsAsync() {
228+
XCTAssertTrue(TreeSitterClient.shouldExecuteAsync(editLength: 0, delta: -500_000, documentLength: 10))
229+
}
230+
}
197231
// swiftlint:enable all

LocalPackages/CodeEditTextView/Sources/CodeEditTextView/TextLayoutManager/TextLayoutManager+Edits.swift

Lines changed: 14 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -80,42 +80,44 @@ extension TextLayoutManager: NSTextStorageDelegate {
8080
/// - Parameter range: The range of the string that was inserted into the text storage.
8181
private func insertNewLines(for range: NSRange) {
8282
guard !range.isEmpty, let string = textStorage?.substring(from: range) as? NSString else { return }
83-
// Loop through each line being inserted, inserting & splitting where necessary
83+
// Loop through each line being inserted, inserting & splitting where necessary. Each line is described by its
84+
// length and whether it ends in a line break rather than a materialized substring, so a large paste does not
85+
// allocate one bridged `String` per line just to test its terminator.
8486
var index = 0
8587
while let nextLine = string.getNextLine(startingAt: index) {
86-
let lineRange = NSRange(start: index, end: nextLine.max)
87-
applyLineInsert(string.substring(with: lineRange) as NSString, at: range.location + index)
88+
applyLineInsert(length: nextLine.max - index, endsInLineBreak: true, at: range.location + index)
8889
index = nextLine.max
8990
}
9091

9192
if index < string.length {
9293
// Get the last line.
93-
applyLineInsert(string.substring(from: index) as NSString, at: range.location + index)
94+
applyLineInsert(length: string.length - index, endsInLineBreak: false, at: range.location + index)
9495
}
9596
}
9697

9798
/// Applies a line insert to the internal line storage tree.
9899
/// - Parameters:
99-
/// - insertedString: The string being inserted.
100+
/// - length: The length of the line being inserted.
101+
/// - endsInLineBreak: Whether the inserted line is terminated by a line break.
100102
/// - location: The location the string is being inserted into.
101-
private func applyLineInsert(_ insertedString: NSString, at location: Int) {
102-
if LineEnding(line: insertedString as String) != nil {
103+
private func applyLineInsert(length: Int, endsInLineBreak: Bool, at location: Int) {
104+
if endsInLineBreak {
103105
if location == lineStorage.length {
104106
// Insert a new line at the end of the document, need to insert a new line 'cause there's nothing to
105107
// split. Also, append the new text to the last line.
106-
lineStorage.update(atOffset: location, delta: insertedString.length, deltaHeight: 0.0)
108+
lineStorage.update(atOffset: location, delta: length, deltaHeight: 0.0)
107109
lineStorage.insert(
108110
line: TextLine(),
109-
atOffset: location + insertedString.length,
111+
atOffset: location + length,
110112
length: 0,
111113
height: estimateLineHeight()
112114
)
113115
} else {
114116
// Need to split the line inserting into and create a new line with the split section of the line
115117
guard let linePosition = lineStorage.getLine(atOffset: location) else { return }
116-
let splitLocation = location + insertedString.length
118+
let splitLocation = location + length
117119
let splitLength = linePosition.range.max - location
118-
let lineDelta = insertedString.length - splitLength // The difference in the line being edited
120+
let lineDelta = length - splitLength // The difference in the line being edited
119121
if lineDelta != 0 {
120122
lineStorage.update(atOffset: location, delta: lineDelta, deltaHeight: 0.0)
121123
}
@@ -128,7 +130,7 @@ extension TextLayoutManager: NSTextStorageDelegate {
128130
)
129131
}
130132
} else {
131-
lineStorage.update(atOffset: location, delta: insertedString.length, deltaHeight: 0.0)
133+
lineStorage.update(atOffset: location, delta: length, deltaHeight: 0.0)
132134
}
133135
}
134136
}

LocalPackages/CodeEditTextView/Tests/CodeEditTextViewTests/LayoutManager/TextLayoutManagerTests.swift

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -269,4 +269,52 @@ struct TextLayoutManagerTests {
269269

270270
#expect(invalidatedLineIds.isSuperset(of: Set(expectedLineIds)))
271271
}
272+
273+
private func makeLaidOutTextView(string: String) -> TextView {
274+
let view = TextView(string: string)
275+
view.frame = NSRect(x: 0, y: 0, width: 1000, height: 1000)
276+
view.updateFrameIfNeeded()
277+
return view
278+
}
279+
280+
/// Pasting a large block of text into a document takes the incremental edit path, while opening a document takes
281+
/// the bulk build path. Both must produce the same line index. This is the scenario behind the editor freezing on
282+
/// a large SQL paste: the incremental path must stay correct after dropping its per-line allocations.
283+
@Test(
284+
arguments: [
285+
("\n", false),
286+
("\n", true),
287+
("\r\n", false),
288+
("\r\n", true),
289+
("\r", false)
290+
]
291+
)
292+
func largePasteMatchesFullRebuild(_ testItem: (String, Bool)) throws {
293+
let (lineBreak, hasTrailingBreak) = testItem
294+
295+
var pasted = (0..<3_000)
296+
.map { "SELECT * FROM table_\($0) WHERE id = \($0);" }
297+
.joined(separator: lineBreak)
298+
if hasTrailingBreak {
299+
pasted += lineBreak
300+
}
301+
302+
let pasteView = makeLaidOutTextView(string: "")
303+
let pasteManager = try #require(pasteView.layoutManager)
304+
pasteView.textStorage.replaceCharacters(in: NSRange(location: 0, length: 0), with: pasted)
305+
pasteManager.lineStorage.validateInternalState()
306+
307+
let oracle = try #require(makeLaidOutTextView(string: pasted).layoutManager)
308+
309+
#expect(pasteManager.lineCount == oracle.lineCount)
310+
#expect(pasteManager.lineStorage.length == oracle.lineStorage.length)
311+
312+
let pastedRanges = (0..<pasteManager.lineCount).compactMap {
313+
pasteManager.lineStorage.getLine(atIndex: $0)?.range
314+
}
315+
let oracleRanges = (0..<oracle.lineCount).compactMap {
316+
oracle.lineStorage.getLine(atIndex: $0)?.range
317+
}
318+
#expect(pastedRanges == oracleRanges)
319+
}
272320
}

0 commit comments

Comments
 (0)