[analysis_server] Improve the performance of applying LSP edits in tests
This slightly improves the performance of applying edits in LSP tests. The previous code would sort the edits in reverse and then sequentially replace each edit into a string. With a large number of edits, both the string replacement and the subsequent rebuilding of LineInfo after each change could be quite slow.
With this change, we instead work through the edits forwards, appending the original text + new text into a StringBuffer to be combined once at the end.
Since most tests don't make large numbers of edits this only shaved a few seconds off the whole server test run, however when running a small benchmark of 20000 edits that Brian sent me recently, the time taken comes down from 42s (which hit the default test timeout) to 17s (which is not fast, but is faster).
Change-Id: Ia91937f4912b35ee1fe29f31157db3ef0e4bd79e
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/429960
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Reviewed-by: Samuel Rawlins <srawlins@google.com>
Commit-Queue: Brian Wilkerson <brianwilkerson@google.com>
diff --git a/pkg/analysis_server/test/lsp/change_verifier.dart b/pkg/analysis_server/test/lsp/change_verifier.dart
index bc65533..5859221 100644
--- a/pkg/analysis_server/test/lsp/change_verifier.dart
+++ b/pkg/analysis_server/test/lsp/change_verifier.dart
@@ -171,19 +171,12 @@
}
String _applyTextDocumentEditEdit(String content, TextDocumentEdit edit) {
- // To simulate the behaviour we'll get from an LSP client, apply edits from
- // the latest offset to the earliest, but with items at the same offset
- // being reversed so that when applied sequentially they appear in the
- // document in-order.
- //
- // This is essentially a stable sort over the offset (descending), but since
- // List.sort() is not stable so we additionally sort by index).
- var indexedEdits =
- edit.edits.mapIndexed(TextEditWithIndex.fromUnion).toList();
- indexedEdits.sort(TextEditWithIndex.compare);
- return indexedEdits
- .map((e) => e.edit)
- .fold(content, editHelpers.applyTextEdit);
+ // Extract the edits from the union (they all have the same superclass).
+ var edits =
+ edit.edits
+ .map((edit) => edit.map((e) => e, (e) => e, (e) => e))
+ .toList();
+ return _applyTextEdits(content, edits);
}
String _applyTextEdits(String content, List<TextEdit> changes) =>
@@ -273,8 +266,8 @@
}
}
-/// An LSP TextEdit with its index, and a comparer to sort them in a way that
-/// can be applied sequentially while preserving expected behaviour.
+/// An LSP TextEdit with its index, and comparers to stably sort them by source
+/// position (forwards).
class TextEditWithIndex {
final int index;
final TextEdit edit;
@@ -286,9 +279,13 @@
Either3<AnnotatedTextEdit, SnippetTextEdit, TextEdit> edit,
) : edit = edit.map((e) => e, (e) => e, (e) => e);
- /// Compares two [TextEditWithIndex] to sort them by the order in which they
- /// can be sequentially applied to a String to match the behaviour of an LSP
- /// client.
+ /// Compares two [TextEditWithIndex] to sort them stably in source-order.
+ ///
+ /// In this order, edits cannot be applied sequentially to a file because
+ /// each edit may change the location of future edits. This can be used to
+ /// apply them if all locations are computed against the original code using
+ /// a [StringBuffer] for better performance than repeatedly applying
+ /// sequentially.
static int compare(TextEditWithIndex edit1, TextEditWithIndex edit2) {
var end1 = edit1.edit.range.end;
var end2 = edit2.edit.range.end;
@@ -297,11 +294,11 @@
// https://github.com/microsoft/vscode/blob/856a306d1a9b0879727421daf21a8059e671e3ea/src/vs/editor/common/model/pieceTreeTextBuffer/pieceTreeTextBuffer.ts#L475
if (end1.line != end2.line) {
- return end1.line.compareTo(end2.line) * -1;
+ return end1.line.compareTo(end2.line);
} else if (end1.character != end2.character) {
- return end1.character.compareTo(end2.character) * -1;
+ return end1.character.compareTo(end2.character);
} else {
- return edit1.index.compareTo(edit2.index) * -1;
+ return edit1.index.compareTo(edit2.index);
}
}
}
diff --git a/pkg/analysis_server/test/lsp/request_helpers_mixin.dart b/pkg/analysis_server/test/lsp/request_helpers_mixin.dart
index e7ed67c..d4538b9 100644
--- a/pkg/analysis_server/test/lsp/request_helpers_mixin.dart
+++ b/pkg/analysis_server/test/lsp/request_helpers_mixin.dart
@@ -24,15 +24,6 @@
/// A mixin with helpers for applying LSP edits to strings.
mixin LspEditHelpersMixin {
- String applyTextEdit(String content, TextEdit edit) {
- var startPos = edit.range.start;
- var endPos = edit.range.end;
- var lineInfo = LineInfo.fromContent(content);
- var start = lineInfo.getOffsetOfLine(startPos.line) + startPos.character;
- var end = lineInfo.getOffsetOfLine(endPos.line) + endPos.character;
- return content.replaceRange(start, end, edit.newText);
- }
-
String applyTextEdits(String content, List<TextEdit> changes) {
// Complex text manipulations are described with an array of TextEdit's,
// representing a single change to the document.
@@ -48,22 +39,48 @@
/// Ensures changes are simple enough to apply easily without any complicated
/// logic.
- void validateChangesCanBeApplied() {
- for (var change1 in changes) {
- for (var change2 in changes) {
- if (change1 != change2 && change1.range.intersects(change2.range)) {
- throw 'Test helper applyTextEdits does not support applying multiple edits '
- 'where the edits are not in reverse order.';
- }
+ for (var change1 in changes) {
+ for (var change2 in changes) {
+ if (change1 != change2 && change1.range.intersects(change2.range)) {
+ throw 'Test helper applyTextEdits does not support applying multiple edits '
+ 'where the edits are not in reverse order.';
}
}
}
- validateChangesCanBeApplied();
-
var indexedEdits = changes.mapIndexed(TextEditWithIndex.new).toList();
indexedEdits.sort(TextEditWithIndex.compare);
- return indexedEdits.map((e) => e.edit).fold(content, applyTextEdit);
+
+ var lineInfo = LineInfo.fromContent(content);
+ var buffer = StringBuffer();
+ var currentOffset = 0;
+
+ // Build a new string by appending parts of the original string and then
+ // the new text from each edit into a buffer. This is faster for multiple
+ // edits than sorting in reverse and repeatedly applying sequentially as it
+ // cuts out a lot of (potentially large) intermediate strings.
+ for (var edit in indexedEdits.map((e) => e.edit)) {
+ var startPos = edit.range.start;
+ var endPos = edit.range.end;
+ var start = lineInfo.getOffsetOfLine(startPos.line) + startPos.character;
+ var end = lineInfo.getOffsetOfLine(endPos.line) + endPos.character;
+
+ // Append any text between the current position and the start of this edit
+ if (start != currentOffset) {
+ buffer.write(content.substring(currentOffset, start));
+ }
+ // Append the replacement text
+ buffer.write(edit.newText);
+ // Move the current position to the end of this edit
+ currentOffset = end;
+ }
+
+ // Finally, add any remainder from after the last edit.
+ if (currentOffset != content.length) {
+ buffer.write(content.substring(currentOffset));
+ }
+
+ return buffer.toString();
}
/// Returns the text for [range] in [content].
diff --git a/pkg/analysis_server/test/lsp_over_legacy/format_test.dart b/pkg/analysis_server/test/lsp_over_legacy/format_test.dart
index 0b4b825..2eac687 100644
--- a/pkg/analysis_server/test/lsp_over_legacy/format_test.dart
+++ b/pkg/analysis_server/test/lsp_over_legacy/format_test.dart
@@ -5,7 +5,6 @@
import 'package:test/test.dart';
import 'package:test_reflective_loader/test_reflective_loader.dart';
-import '../lsp/request_helpers_mixin.dart';
import 'abstract_lsp_over_legacy.dart';
void main() {
@@ -15,7 +14,7 @@
}
@reflectiveTest
-class FormatTest extends LspOverLegacyTest with LspEditHelpersMixin {
+class FormatTest extends LspOverLegacyTest {
Future<void> test_format() async {
const content = 'void main() {}';
const expectedContent = 'void main() {}';
