Skip to content

Commit 0d4b08c

Browse files
jensjohaCommit Queue
authored andcommitted
[analyzer] Use QueueList by default in SourceFileEdit
Adding edits (e.g. via `dart fix --apply` are often done with `List.insert(0, whatnot)` which takes O(n) time. Here QueueList that can insert fast at both ends is used instead (and we use `addFirst` instead of `insert(0)`. On the example from https://github.com/feinstein/google-i18n-address-dart.git we go from: ``` $ time dart fix --use-aot-snapshot --apply [...] 249517 fixes made in 255 files. real 3m55.810s user 4m1.209s sys 0m3.714s (resetting) $ time dart fix --use-aot-snapshot --apply [...] 249517 fixes made in 255 files. real 3m33.966s user 3m37.588s sys 0m2.058s (resetting) $ time dart fix --use-aot-snapshot --apply [...] 249517 fixes made in 255 files. real 3m36.525s user 3m40.083s sys 0m1.907s ``` to: ``` $ time dart fix --use-aot-snapshot --apply [...] 249517 fixes made in 255 files. real 0m9.970s user 0m12.676s sys 0m2.100s (resetting) $ time dart fix --use-aot-snapshot --apply [...] 249517 fixes made in 255 files. real 0m9.862s user 0m12.926s sys 0m1.797s (resetting) $ time dart fix --use-aot-snapshot --apply [...] 249517 fixes made in 255 files. real 0m9.612s user 0m12.712s sys 0m1.834s ``` Statistics on the `real` runtime: ``` N Min Max Median Avg Stddev x 3 213.966 235.81 216.525 222.10033 11.941664 + 3 9.612 9.97 9.862 9.8146667 0.18363369 Difference at 95.0% confidence -212.286 +/- 19.1415 -95.581% +/- 8.61838% (Student's t, pooled s = 8.44503) ``` For `lsp_many_prefer_single_quotes_violations_benchmark.dart --sizes=3200`: Before from something like: ``` Initial analysis: 0.115654 First code action call: 0.835152 Subsequent action call 1: 0.538592 Subsequent action call 2: 0.561636 Select all code action call: 1.564402 ``` After to something like: ``` Initial analysis: 0.086985 First code action call: 0.411660 Subsequent action call 1: 0.171566 Subsequent action call 2: 0.193708 Select all code action call: 1.107339 ``` Statistics on 5 runs gives: First code action call: ``` Difference at 95.0% confidence -0.44381 +/- 0.0261597 -52.4602% +/- 3.09218% (Student's t, pooled s = 0.0179367) ``` Subsequent action call 1: ``` Difference at 95.0% confidence -0.381012 +/- 0.0195618 -69.9139% +/- 3.5895% (Student's t, pooled s = 0.0134128) ``` Subsequent action call 2: ``` Difference at 95.0% confidence -0.360077 +/- 0.0265277 -64.634% +/- 4.76173% (Student's t, pooled s = 0.0181891) ``` Select all code action call: ``` Difference at 95.0% confidence -0.405855 +/- 0.027662 -26.7277% +/- 1.82169% (Student's t, pooled s = 0.0189668) ``` Change-Id: I3868afaa8c32a24c01c3a52bd8a53d5e8e4e3afe Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/427401 Commit-Queue: Jens Johansen <[email protected]> Reviewed-by: Brian Wilkerson <[email protected]>
1 parent 1f26ddf commit 0d4b08c

File tree

7 files changed

+33
-5
lines changed

7 files changed

+33
-5
lines changed

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

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,8 @@
88

99
import 'dart:convert' hide JsonDecoder;
1010

11+
import 'package:collection/collection.dart' show QueueList;
12+
1113
import 'package:analysis_server_client/src/protocol/protocol_internal.dart';
1214

1315
// ignore_for_file: flutter_style_todos
@@ -3678,7 +3680,7 @@ class SourceFileEdit implements HasToJson {
36783680
List<SourceEdit> edits;
36793681

36803682
SourceFileEdit(this.file, this.fileStamp, {List<SourceEdit>? edits})
3681-
: edits = edits ?? <SourceEdit>[];
3683+
: edits = edits ?? QueueList<SourceEdit>();
36823684

36833685
factory SourceFileEdit.fromJson(
36843686
JsonDecoder jsonDecoder, String jsonPath, Object? json) {

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

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,12 @@ void addEditForSource(SourceFileEdit sourceFileEdit, SourceEdit sourceEdit,
4545
index++;
4646
}
4747
}
48-
edits.insert(index, sourceEdit);
48+
if (index == 0 && edits is Queue) {
49+
var q = edits as Queue;
50+
q.addFirst(sourceEdit);
51+
} else {
52+
edits.insert(index, sourceEdit);
53+
}
4954
}
5055

5156
/// Adds [edit] to the [FileEdit] for the given [file].

pkg/analysis_server_client/pubspec.yaml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ resolution: workspace
1616

1717
# Use 'any' constraints here; we get our versions from the DEPS file.
1818
dependencies:
19+
collection: any
1920
path: any
2021
pub_semver: any
2122

pkg/analyzer_plugin/lib/protocol/protocol_common.dart

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,8 @@
88

99
import 'dart:convert' hide JsonDecoder;
1010

11+
import 'package:collection/collection.dart' show QueueList;
12+
1113
import 'package:analyzer_plugin/src/protocol/protocol_internal.dart';
1214
import 'package:analyzer_plugin/src/utilities/client_uri_converter.dart';
1315

@@ -3759,7 +3761,7 @@ class SourceFileEdit implements HasToJson {
37593761
List<SourceEdit> edits;
37603762

37613763
SourceFileEdit(this.file, this.fileStamp, {List<SourceEdit>? edits})
3762-
: edits = edits ?? <SourceEdit>[];
3764+
: edits = edits ?? QueueList<SourceEdit>();
37633765

37643766
factory SourceFileEdit.fromJson(
37653767
JsonDecoder jsonDecoder, String jsonPath, Object? json,

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

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,12 @@ void addEditForSource(SourceFileEdit sourceFileEdit, SourceEdit sourceEdit,
7878
newEdit: sourceEdit, existingEdit: nextEdit);
7979
}
8080
}
81-
edits.insert(index, sourceEdit);
81+
if (index == 0 && edits is Queue) {
82+
var q = edits as Queue;
83+
q.addFirst(sourceEdit);
84+
} else {
85+
edits.insert(index, sourceEdit);
86+
}
8287
}
8388

8489
/// Adds [edit] to the [FileEdit] for the given [file].

pkg/analyzer_plugin/tool/spec/codegen_dart_protocol.dart

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,12 @@ class CodegenProtocolVisitor extends DartCodegenVisitor with CodeGenerator {
5454
'TypeHierarchyItem': ['interfaces', 'mixins', 'subclasses'],
5555
};
5656

57+
/// Class members for which the list type should not be the default,
58+
/// but QueueList for performance reasons.
59+
static const Map<String, List<String>> _useQueueList = {
60+
'SourceFileEdit': ['edits'],
61+
};
62+
5763
/// The disclaimer added to the documentation comment for each of the classes
5864
/// that are generated.
5965
static const String disclaimer =
@@ -461,7 +467,12 @@ class CodegenProtocolVisitor extends DartCodegenVisitor with CodeGenerator {
461467
// given, the constructor should populate with the empty list.
462468
var fieldType = field.type;
463469
if (fieldType is TypeList) {
464-
var defaultValue = '<${dartType(fieldType.itemType)}>[]';
470+
String defaultValue;
471+
if (_useQueueList[className]?.contains(field.name) ?? false) {
472+
defaultValue = 'QueueList<${dartType(fieldType.itemType)}>()';
473+
} else {
474+
defaultValue = '<${dartType(fieldType.itemType)}>[]';
475+
}
465476
initializers.add('${field.name} = ${field.name} ?? $defaultValue');
466477
} else {
467478
throw Exception("Don't know how to create default field value.");

pkg/analyzer_plugin/tool/spec/codegen_protocol_common.dart

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,8 @@ class CodegenCommonVisitor extends CodegenProtocolVisitor {
4545
void emitImports() {
4646
writeln("import 'dart:convert' hide JsonDecoder;");
4747
writeln();
48+
writeln("import 'package:collection/collection.dart' show QueueList;");
49+
writeln();
4850
if (forClient) {
4951
writeln(
5052
"import 'package:analysis_server_client/src/protocol/protocol_internal.dart';");

0 commit comments

Comments
 (0)