Skip to content

Commit 24cae8a

Browse files
bwilkersonCommit Queue
authored andcommitted
Improve the performance of applying edits
I wrote a test that generates 10,000 lines of code, each containing one violation of `prefer_single_quotes`, computes the fixes for all of the diagnostics, and applies the 20,000 edits, measuring how long the application takes. Before this change the average was 3355.0 ms. After this change the average is 3.8 ms. The change was inspired by Jens. Change-Id: Icd54a81ca3848fd5f0168934f713a7be8caec359 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/425601 Reviewed-by: Samuel Rawlins <[email protected]> Commit-Queue: Brian Wilkerson <[email protected]>
1 parent 2547ccd commit 24cae8a

File tree

9 files changed

+108
-18
lines changed

9 files changed

+108
-18
lines changed

pkg/analysis_server/lib/src/protocol/protocol_internal.dart

Lines changed: 34 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -23,11 +23,41 @@ final Map<String, RefactoringKind> REQUEST_ID_REFACTORING_KINDS =
2323
/// Get the result of applying a set of [edits] to the given [code]. Edits
2424
/// are applied in the order they appear in [edits]. Access via
2525
/// SourceEdit.applySequence().
26-
String applySequenceOfEdits(String code, Iterable<SourceEdit> edits) {
27-
for (var edit in edits) {
28-
code = edit.apply(code);
26+
String applySequenceOfEdits(String code, List<SourceEdit> edits) {
27+
var buffer = StringBuffer();
28+
var start = 0;
29+
for (var i = edits.length - 1; i >= 0; i--) {
30+
var edit = edits[i];
31+
var offset = edit.offset;
32+
var length = edit.length;
33+
if (length < 0) {
34+
throw RangeError('The edit length is negative.');
35+
}
36+
if (offset + length > code.length) {
37+
throw RangeError('The edit extends past the end of the code.');
38+
}
39+
if (start > offset) {
40+
// One of the edits overlaps with another, requiring that they be applied
41+
// from largest offset to smallest. This should only be possible in code
42+
// that creates source edits without using the `ChangeBuilder` to do so.
43+
//
44+
// We should consider fixing the places where overlapping edits are
45+
// produced so that this branch can be removed. One such place is
46+
// exhibited by `_DoCompletionTest.test_noBody`.
47+
for (var edit in edits) {
48+
code = edit.apply(code);
49+
}
50+
return code;
51+
} else if (start < offset) {
52+
buffer.write(code.substring(start, offset));
53+
}
54+
buffer.write(edit.replacement);
55+
start = offset + length;
56+
}
57+
if (start < code.length) {
58+
buffer.write(code.substring(start));
2959
}
30-
return code;
60+
return buffer.toString();
3161
}
3262

3363
/// Compare the lists [listA] and [listB], using [itemEqual] to compare

pkg/analysis_server/lib/src/services/correction/dart/flutter_convert_to_stateful_widget.dart

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -113,7 +113,7 @@ class FlutterConvertToStatefulWidget extends ResolvedCorrectionProducer {
113113
linesRange,
114114
);
115115
movedNode.accept(visitor);
116-
return SourceEdit.applySequence(text, visitor.edits.reversed);
116+
return SourceEdit.applySequence(text, visitor.edits.reversed.toList());
117117
}
118118

119119
var statefulWidgetClass = await sessionHelper.getFlutterClass(

pkg/analysis_server/lib/src/services/correction/dart/flutter_convert_to_stateless_widget.dart

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -135,7 +135,7 @@ class FlutterConvertToStatelessWidget extends ResolvedCorrectionProducer {
135135
linesRange,
136136
);
137137
movedNode.accept(visitor);
138-
return SourceEdit.applySequence(text, visitor.edits.reversed);
138+
return SourceEdit.applySequence(text, visitor.edits.reversed.toList());
139139
}
140140

141141
var statelessWidgetClass = await sessionHelper.getFlutterClass(

pkg/analysis_server/tool/spec/codegen_dart_protocol.dart

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -903,7 +903,7 @@ class CodegenProtocolVisitor extends DartCodegenVisitor with CodeGenerator {
903903
),
904904
]);
905905
writeln(
906-
'static String applySequence(String code, Iterable<SourceEdit> edits) =>',
906+
'static String applySequence(String code, List<SourceEdit> edits) =>',
907907
);
908908
writeln(' applySequenceOfEdits(code, edits);');
909909
return true;

pkg/analysis_server_client/lib/src/protocol/protocol_common.dart

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3489,7 +3489,7 @@ class SourceChange implements HasToJson {
34893489
class SourceEdit implements HasToJson {
34903490
/// Get the result of applying a set of [edits] to the given [code]. Edits
34913491
/// are applied in the order they appear in [edits].
3492-
static String applySequence(String code, Iterable<SourceEdit> edits) =>
3492+
static String applySequence(String code, List<SourceEdit> edits) =>
34933493
applySequenceOfEdits(code, edits);
34943494

34953495
/// The offset of the region to be modified.

pkg/analysis_server_client/lib/src/protocol/protocol_internal.dart

Lines changed: 34 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -76,11 +76,41 @@ String applyEdit(String code, SourceEdit edit) {
7676
/// Get the result of applying a set of [edits] to the given [code]. Edits
7777
/// are applied in the order they appear in [edits]. Access via
7878
/// SourceEdit.applySequence().
79-
String applySequenceOfEdits(String code, Iterable<SourceEdit> edits) {
80-
for (var edit in edits) {
81-
code = edit.apply(code);
79+
String applySequenceOfEdits(String code, List<SourceEdit> edits) {
80+
var buffer = StringBuffer();
81+
var start = 0;
82+
for (var i = edits.length - 1; i >= 0; i--) {
83+
var edit = edits[i];
84+
var offset = edit.offset;
85+
var length = edit.length;
86+
if (length < 0) {
87+
throw RangeError('The edit length is negative.');
88+
}
89+
if (offset + length > code.length) {
90+
throw RangeError('The edit extends past the end of the code.');
91+
}
92+
if (start > offset) {
93+
// One of the edits overlaps with another, requiring that they be applied
94+
// from largest offset to smallest. This should only be possible in code
95+
// that creates source edits without using the `ChangeBuilder` to do so.
96+
//
97+
// We should consider fixing the places where overlapping edits are
98+
// produced so that this branch can be removed. One such place is
99+
// exhibited by `_DoCompletionTest.test_noBody`.
100+
for (var edit in edits) {
101+
code = edit.apply(code);
102+
}
103+
return code;
104+
} else if (start < offset) {
105+
buffer.write(code.substring(start, offset));
106+
}
107+
buffer.write(edit.replacement);
108+
start = offset + length;
109+
}
110+
if (start < code.length) {
111+
buffer.write(code.substring(start));
82112
}
83-
return code;
113+
return buffer.toString();
84114
}
85115

86116
/// Returns the [FileEdit] for the given [file], maybe `null`.

pkg/analyzer_plugin/lib/protocol/protocol_common.dart

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3569,7 +3569,7 @@ class SourceChange implements HasToJson {
35693569
class SourceEdit implements HasToJson {
35703570
/// Get the result of applying a set of [edits] to the given [code]. Edits
35713571
/// are applied in the order they appear in [edits].
3572-
static String applySequence(String code, Iterable<SourceEdit> edits) =>
3572+
static String applySequence(String code, List<SourceEdit> edits) =>
35733573
applySequenceOfEdits(code, edits);
35743574

35753575
/// The offset of the region to be modified.

pkg/analyzer_plugin/lib/src/protocol/protocol_internal.dart

Lines changed: 34 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -109,11 +109,41 @@ String applyEdit(String code, SourceEdit edit) {
109109
/// Get the result of applying a set of [edits] to the given [code]. Edits
110110
/// are applied in the order they appear in [edits]. Access via
111111
/// SourceEdit.applySequence().
112-
String applySequenceOfEdits(String code, Iterable<SourceEdit> edits) {
113-
for (var edit in edits) {
114-
code = edit.apply(code);
112+
String applySequenceOfEdits(String code, List<SourceEdit> edits) {
113+
var buffer = StringBuffer();
114+
var start = 0;
115+
for (var i = edits.length - 1; i >= 0; i--) {
116+
var edit = edits[i];
117+
var offset = edit.offset;
118+
var length = edit.length;
119+
if (length < 0) {
120+
throw RangeError('The edit length is negative.');
121+
}
122+
if (offset + length > code.length) {
123+
throw RangeError('The edit extends past the end of the code.');
124+
}
125+
if (start > offset) {
126+
// One of the edits overlaps with another, requiring that they be applied
127+
// from largest offset to smallest. This should only be possible in code
128+
// that creates source edits without using the `ChangeBuilder` to do so.
129+
//
130+
// We should consider fixing the places where overlapping edits are
131+
// produced so that this branch can be removed. One such place is
132+
// exhibited by `_DoCompletionTest.test_noBody`.
133+
for (var edit in edits) {
134+
code = edit.apply(code);
135+
}
136+
return code;
137+
} else if (start < offset) {
138+
buffer.write(code.substring(start, offset));
139+
}
140+
buffer.write(edit.replacement);
141+
start = offset + length;
142+
}
143+
if (start < code.length) {
144+
buffer.write(code.substring(start));
115145
}
116-
return code;
146+
return buffer.toString();
117147
}
118148

119149
/// Returns the [FileEdit] for the given [file], maybe `null`.

pkg/analyzer_plugin/tool/spec/codegen_dart_protocol.dart

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -839,7 +839,7 @@ class CodegenProtocolVisitor extends DartCodegenVisitor with CodeGenerator {
839839
'[code]. Edits are applied in the order they appear in [edits].')
840840
]);
841841
writeln(
842-
'static String applySequence(String code, Iterable<SourceEdit> edits) =>');
842+
'static String applySequence(String code, List<SourceEdit> edits) =>');
843843
writeln(' applySequenceOfEdits(code, edits);');
844844
return true;
845845
default:

0 commit comments

Comments
 (0)