From 0d4b08caaa711b19903563f3abb9ec436db6eeab Mon Sep 17 00:00:00 2001 From: Jens Johansen Date: Fri, 16 May 2025 00:45:34 -0700 Subject: [PATCH] [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 Reviewed-by: Brian Wilkerson --- .../lib/src/protocol/protocol_common.dart | 4 +++- .../lib/src/protocol/protocol_internal.dart | 7 ++++++- pkg/analysis_server_client/pubspec.yaml | 1 + .../lib/protocol/protocol_common.dart | 4 +++- .../lib/src/protocol/protocol_internal.dart | 7 ++++++- .../tool/spec/codegen_dart_protocol.dart | 13 ++++++++++++- .../tool/spec/codegen_protocol_common.dart | 2 ++ 7 files changed, 33 insertions(+), 5 deletions(-) diff --git a/pkg/analysis_server_client/lib/src/protocol/protocol_common.dart b/pkg/analysis_server_client/lib/src/protocol/protocol_common.dart index 0da011a1315..8bdc1bdc10e 100644 --- a/pkg/analysis_server_client/lib/src/protocol/protocol_common.dart +++ b/pkg/analysis_server_client/lib/src/protocol/protocol_common.dart @@ -8,6 +8,8 @@ import 'dart:convert' hide JsonDecoder; +import 'package:collection/collection.dart' show QueueList; + import 'package:analysis_server_client/src/protocol/protocol_internal.dart'; // ignore_for_file: flutter_style_todos @@ -3678,7 +3680,7 @@ class SourceFileEdit implements HasToJson { List edits; SourceFileEdit(this.file, this.fileStamp, {List? edits}) - : edits = edits ?? []; + : edits = edits ?? QueueList(); factory SourceFileEdit.fromJson( JsonDecoder jsonDecoder, String jsonPath, Object? json) { diff --git a/pkg/analysis_server_client/lib/src/protocol/protocol_internal.dart b/pkg/analysis_server_client/lib/src/protocol/protocol_internal.dart index e941230ccb7..27c7d668422 100644 --- a/pkg/analysis_server_client/lib/src/protocol/protocol_internal.dart +++ b/pkg/analysis_server_client/lib/src/protocol/protocol_internal.dart @@ -45,7 +45,12 @@ void addEditForSource(SourceFileEdit sourceFileEdit, SourceEdit sourceEdit, index++; } } - edits.insert(index, sourceEdit); + if (index == 0 && edits is Queue) { + var q = edits as Queue; + q.addFirst(sourceEdit); + } else { + edits.insert(index, sourceEdit); + } } /// Adds [edit] to the [FileEdit] for the given [file]. diff --git a/pkg/analysis_server_client/pubspec.yaml b/pkg/analysis_server_client/pubspec.yaml index 38755091a24..80895ebc09a 100644 --- a/pkg/analysis_server_client/pubspec.yaml +++ b/pkg/analysis_server_client/pubspec.yaml @@ -16,6 +16,7 @@ resolution: workspace # Use 'any' constraints here; we get our versions from the DEPS file. dependencies: + collection: any path: any pub_semver: any diff --git a/pkg/analyzer_plugin/lib/protocol/protocol_common.dart b/pkg/analyzer_plugin/lib/protocol/protocol_common.dart index 6a3cacdfe26..5e9f448d70f 100644 --- a/pkg/analyzer_plugin/lib/protocol/protocol_common.dart +++ b/pkg/analyzer_plugin/lib/protocol/protocol_common.dart @@ -8,6 +8,8 @@ import 'dart:convert' hide JsonDecoder; +import 'package:collection/collection.dart' show QueueList; + import 'package:analyzer_plugin/src/protocol/protocol_internal.dart'; import 'package:analyzer_plugin/src/utilities/client_uri_converter.dart'; @@ -3759,7 +3761,7 @@ class SourceFileEdit implements HasToJson { List edits; SourceFileEdit(this.file, this.fileStamp, {List? edits}) - : edits = edits ?? []; + : edits = edits ?? QueueList(); factory SourceFileEdit.fromJson( JsonDecoder jsonDecoder, String jsonPath, Object? json, diff --git a/pkg/analyzer_plugin/lib/src/protocol/protocol_internal.dart b/pkg/analyzer_plugin/lib/src/protocol/protocol_internal.dart index 0d9e8727e68..ca90d437352 100644 --- a/pkg/analyzer_plugin/lib/src/protocol/protocol_internal.dart +++ b/pkg/analyzer_plugin/lib/src/protocol/protocol_internal.dart @@ -78,7 +78,12 @@ void addEditForSource(SourceFileEdit sourceFileEdit, SourceEdit sourceEdit, newEdit: sourceEdit, existingEdit: nextEdit); } } - edits.insert(index, sourceEdit); + if (index == 0 && edits is Queue) { + var q = edits as Queue; + q.addFirst(sourceEdit); + } else { + edits.insert(index, sourceEdit); + } } /// Adds [edit] to the [FileEdit] for the given [file]. diff --git a/pkg/analyzer_plugin/tool/spec/codegen_dart_protocol.dart b/pkg/analyzer_plugin/tool/spec/codegen_dart_protocol.dart index fdb60a04240..c1ab2779506 100644 --- a/pkg/analyzer_plugin/tool/spec/codegen_dart_protocol.dart +++ b/pkg/analyzer_plugin/tool/spec/codegen_dart_protocol.dart @@ -54,6 +54,12 @@ class CodegenProtocolVisitor extends DartCodegenVisitor with CodeGenerator { 'TypeHierarchyItem': ['interfaces', 'mixins', 'subclasses'], }; + /// Class members for which the list type should not be the default, + /// but QueueList for performance reasons. + static const Map> _useQueueList = { + 'SourceFileEdit': ['edits'], + }; + /// The disclaimer added to the documentation comment for each of the classes /// that are generated. static const String disclaimer = @@ -461,7 +467,12 @@ class CodegenProtocolVisitor extends DartCodegenVisitor with CodeGenerator { // given, the constructor should populate with the empty list. var fieldType = field.type; if (fieldType is TypeList) { - var defaultValue = '<${dartType(fieldType.itemType)}>[]'; + String defaultValue; + if (_useQueueList[className]?.contains(field.name) ?? false) { + defaultValue = 'QueueList<${dartType(fieldType.itemType)}>()'; + } else { + defaultValue = '<${dartType(fieldType.itemType)}>[]'; + } initializers.add('${field.name} = ${field.name} ?? $defaultValue'); } else { throw Exception("Don't know how to create default field value."); diff --git a/pkg/analyzer_plugin/tool/spec/codegen_protocol_common.dart b/pkg/analyzer_plugin/tool/spec/codegen_protocol_common.dart index 2e00137cef1..e13b31497ff 100644 --- a/pkg/analyzer_plugin/tool/spec/codegen_protocol_common.dart +++ b/pkg/analyzer_plugin/tool/spec/codegen_protocol_common.dart @@ -45,6 +45,8 @@ class CodegenCommonVisitor extends CodegenProtocolVisitor { void emitImports() { writeln("import 'dart:convert' hide JsonDecoder;"); writeln(); + writeln("import 'package:collection/collection.dart' show QueueList;"); + writeln(); if (forClient) { writeln( "import 'package:analysis_server_client/src/protocol/protocol_internal.dart';");