[analyzer] Add "interactive" option to "getResolvedUnit"; "setSubscriptions" are not; non-interactive ones have less priority
The legacy_many_files_in_flutter_set_subscriptions benchmark shows how
"flutter.setSubscriptions" calls can make the analyzer slower to
respond.
What happens is this:
* The user opens a new file in the IDE.
* The IDE sends the `flutter.setSubscriptions` request which equates to
a call to `getResolvedUnit` for each file in the request. If this is,
say, 300 files it's 300 calls to `getResolvedUnit`.
* The IDE sends a `edit.getAssists` request for the newly opened file.
This request starts processing, reaches `getResolvedLibrary(file)`
which calls `getUnitElement` ultimately adding the path to
`_unitElementRequestedFiles` which in `performWork` is done _after_
`_requestedFiles`, meaning it has to do all the flutter requested
files first.
* The user might then request completion for instance, but because the
analyzer only processes one request at a time it has to wait for the
`edit.getAssists` request to finish first, which had to wait for the
files from the `flutter.setSubscriptions` request to process.
All in all it's a lot of waiting for the user.
This CL adds a `interactive` option to the `getResolvedUnit` call. It
defaults to true in which case files are still added to
`_requestedFiles` and processed the same. If it's false it will instead
be added to a newly introduced list instead and processed at a lower
priority. Subscription requests are changed to pass `false` to
`interactive`, avoiding the scenario above.
Comparing before this CL with this CL on the
"legacy_many_files_in_flutter_set_subscriptions" benchmark with 100
files / CodeType.ImportChain these are the statistics on the changes
based on 5 runs each:
```
Completion after open of new file: -81.6652% +/- 7.7564% (-3.70 +/- 0.35) (4.53 -> 0.83)
getAssists call: -96.6315% +/- 0.9307% (-3.61 +/- 0.03) (3.74 -> 0.13)
peak virtual memory size: -5.6786% +/- 3.2964% (-139.00 +/- 80.69) (2447.80 -> 2308.80)
total program size (virtual): -4.6387% +/- 3.8146% (-110.80 +/- 91.11) (2388.60 -> 2277.80)
```
Even when https://github.com/flutter/flutter-intellij/issues/7980 is
hopefully fixed I think it is a fair change to de-prioritize a
non-interactive request.
Change-Id: Icba2faebf12f9913cf24db7cb90fdc6f4c74164e
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/418020
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Jens Johansen <jensj@google.com>
diff --git a/pkg/analysis_server/lib/src/analysis_server.dart b/pkg/analysis_server/lib/src/analysis_server.dart
index 1e210c5..1ec1ec5 100644
--- a/pkg/analysis_server/lib/src/analysis_server.dart
+++ b/pkg/analysis_server/lib/src/analysis_server.dart
@@ -793,6 +793,7 @@
Future<ResolvedUnitResult?>? getResolvedUnit(
String path, {
bool sendCachedToStream = false,
+ bool interactive = true,
}) {
if (!file_paths.isDart(resourceProvider.pathContext, path)) {
return null;
@@ -804,7 +805,11 @@
}
return driver
- .getResolvedUnit(path, sendCachedToStream: sendCachedToStream)
+ .getResolvedUnit(
+ path,
+ sendCachedToStream: sendCachedToStream,
+ interactive: interactive,
+ )
.then((value) => value is ResolvedUnitResult ? value : null)
.catchError((Object e, StackTrace st) {
instrumentationService.logException(e, st);
diff --git a/pkg/analysis_server/lib/src/legacy_analysis_server.dart b/pkg/analysis_server/lib/src/legacy_analysis_server.dart
index 0717609..d66349c 100644
--- a/pkg/analysis_server/lib/src/legacy_analysis_server.dart
+++ b/pkg/analysis_server/lib/src/legacy_analysis_server.dart
@@ -1190,7 +1190,7 @@
// the fully resolved unit, and processed with sending analysis
// notifications as it happens after content changes.
if (file_paths.isDart(resourceProvider.pathContext, file)) {
- getResolvedUnit(file, sendCachedToStream: true);
+ getResolvedUnit(file, sendCachedToStream: true, interactive: false);
}
}
}
diff --git a/pkg/analyzer/lib/src/dart/analysis/driver.dart b/pkg/analyzer/lib/src/dart/analysis/driver.dart
index e5f86fc..eee4eb2 100644
--- a/pkg/analyzer/lib/src/dart/analysis/driver.dart
+++ b/pkg/analyzer/lib/src/dart/analysis/driver.dart
@@ -183,6 +183,12 @@
final _requestedFiles = <String, List<Completer<SomeResolvedUnitResult>>>{};
/// The mapping from the files for which analysis was requested using
+ /// [getResolvedUnit] which are not interactive to the [Completer]s to report
+ /// the result.
+ final _requestedFilesNonInteractive =
+ <String, List<Completer<SomeResolvedUnitResult>>>{};
+
+ /// The mapping from the files for which analysis was requested using
/// [getResolvedLibrary] to the [Completer]s to report the result.
final _requestedLibraries =
<String, List<Completer<SomeResolvedLibraryResult>>>{};
@@ -494,6 +500,10 @@
}
}
}
+
+ if (_requestedFilesNonInteractive.isNotEmpty) {
+ return AnalysisDriverPriority.general;
+ }
if (_pendingFileChanges.isNotEmpty) {
return AnalysisDriverPriority.general;
}
@@ -525,6 +535,7 @@
_fileTracker.hasChangedFiles ||
_requestedLibraries.isNotEmpty ||
_requestedFiles.isNotEmpty ||
+ _requestedFilesNonInteractive.isNotEmpty ||
_errorsRequestedFiles.isNotEmpty ||
_definingClassMemberNameRequests.isNotEmpty ||
_referencingNameRequests.isNotEmpty ||
@@ -736,6 +747,13 @@
}
_requestedFiles.clear();
+ for (var completerList in _requestedFilesNonInteractive.values) {
+ for (var completer in completerList) {
+ completer.complete(DisposedAnalysisContextResult());
+ }
+ }
+ _requestedFilesNonInteractive.clear();
+
for (var completerList in _unitElementRequestedFiles.values) {
for (var completer in completerList) {
completer.complete(DisposedAnalysisContextResult());
@@ -1059,7 +1077,7 @@
/// of the files previously reported using [changeFile]), prior to the next
/// time the analysis state transitions to "idle".
Future<SomeResolvedUnitResult> getResolvedUnit(String path,
- {bool sendCachedToStream = false}) {
+ {bool sendCachedToStream = false, bool interactive = true}) {
if (!_isAbsolutePath(path)) {
return Future.value(
InvalidPathResult(),
@@ -1091,7 +1109,11 @@
// Schedule analysis.
var completer = Completer<SomeResolvedUnitResult>();
- _requestedFiles.add(path, completer);
+ if (interactive) {
+ _requestedFiles.add(path, completer);
+ } else {
+ _requestedFilesNonInteractive.add(path, completer);
+ }
_scheduler.notify();
return completer.future;
}
@@ -1245,6 +1267,12 @@
_produceErrors(path);
return;
}
+
+ // Analyze a requested - but not interactivly requested - file.
+ if (_requestedFilesNonInteractive.firstKey case var path?) {
+ _analyzeFile(path);
+ return;
+ }
}
/// Remove the file with the given [path] from the list of files to analyze.
@@ -1412,6 +1440,8 @@
// getResolvedUnit()
_requestedFiles.completeAll(unitFile.path, resolvedUnit);
+ _requestedFilesNonInteractive.completeAll(
+ unitFile.path, resolvedUnit);
// getErrors()
_errorsRequestedFiles.completeAll(
@@ -1517,6 +1547,9 @@
completeWithError(
_requestedFiles.remove(file.path),
);
+ completeWithError(
+ _requestedFilesNonInteractive.remove(file.path),
+ );
// getErrors()
completeWithError(
_errorsRequestedFiles.remove(file.path),
