Skip to content

Commit 9f42ef7

Browse files
jensjohaCommit Queue
authored andcommitted
[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 flutter/flutter-intellij#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 <[email protected]> Commit-Queue: Jens Johansen <[email protected]>
1 parent f6fcd37 commit 9f42ef7

File tree

3 files changed

+42
-4
lines changed

3 files changed

+42
-4
lines changed

pkg/analysis_server/lib/src/analysis_server.dart

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -793,6 +793,7 @@ abstract class AnalysisServer {
793793
Future<ResolvedUnitResult?>? getResolvedUnit(
794794
String path, {
795795
bool sendCachedToStream = false,
796+
bool interactive = true,
796797
}) {
797798
if (!file_paths.isDart(resourceProvider.pathContext, path)) {
798799
return null;
@@ -804,7 +805,11 @@ abstract class AnalysisServer {
804805
}
805806

806807
return driver
807-
.getResolvedUnit(path, sendCachedToStream: sendCachedToStream)
808+
.getResolvedUnit(
809+
path,
810+
sendCachedToStream: sendCachedToStream,
811+
interactive: interactive,
812+
)
808813
.then((value) => value is ResolvedUnitResult ? value : null)
809814
.catchError((Object e, StackTrace st) {
810815
instrumentationService.logException(e, st);

pkg/analysis_server/lib/src/legacy_analysis_server.dart

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1190,7 +1190,7 @@ class LegacyAnalysisServer extends AnalysisServer {
11901190
// the fully resolved unit, and processed with sending analysis
11911191
// notifications as it happens after content changes.
11921192
if (file_paths.isDart(resourceProvider.pathContext, file)) {
1193-
getResolvedUnit(file, sendCachedToStream: true);
1193+
getResolvedUnit(file, sendCachedToStream: true, interactive: false);
11941194
}
11951195
}
11961196
}

pkg/analyzer/lib/src/dart/analysis/driver.dart

Lines changed: 35 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -182,6 +182,12 @@ class AnalysisDriver {
182182
/// [getResolvedUnit] to the [Completer]s to report the result.
183183
final _requestedFiles = <String, List<Completer<SomeResolvedUnitResult>>>{};
184184

185+
/// The mapping from the files for which analysis was requested using
186+
/// [getResolvedUnit] which are not interactive to the [Completer]s to report
187+
/// the result.
188+
final _requestedFilesNonInteractive =
189+
<String, List<Completer<SomeResolvedUnitResult>>>{};
190+
185191
/// The mapping from the files for which analysis was requested using
186192
/// [getResolvedLibrary] to the [Completer]s to report the result.
187193
final _requestedLibraries =
@@ -494,6 +500,10 @@ class AnalysisDriver {
494500
}
495501
}
496502
}
503+
504+
if (_requestedFilesNonInteractive.isNotEmpty) {
505+
return AnalysisDriverPriority.general;
506+
}
497507
if (_pendingFileChanges.isNotEmpty) {
498508
return AnalysisDriverPriority.general;
499509
}
@@ -525,6 +535,7 @@ class AnalysisDriver {
525535
_fileTracker.hasChangedFiles ||
526536
_requestedLibraries.isNotEmpty ||
527537
_requestedFiles.isNotEmpty ||
538+
_requestedFilesNonInteractive.isNotEmpty ||
528539
_errorsRequestedFiles.isNotEmpty ||
529540
_definingClassMemberNameRequests.isNotEmpty ||
530541
_referencingNameRequests.isNotEmpty ||
@@ -736,6 +747,13 @@ class AnalysisDriver {
736747
}
737748
_requestedFiles.clear();
738749

750+
for (var completerList in _requestedFilesNonInteractive.values) {
751+
for (var completer in completerList) {
752+
completer.complete(DisposedAnalysisContextResult());
753+
}
754+
}
755+
_requestedFilesNonInteractive.clear();
756+
739757
for (var completerList in _unitElementRequestedFiles.values) {
740758
for (var completer in completerList) {
741759
completer.complete(DisposedAnalysisContextResult());
@@ -1059,7 +1077,7 @@ class AnalysisDriver {
10591077
/// of the files previously reported using [changeFile]), prior to the next
10601078
/// time the analysis state transitions to "idle".
10611079
Future<SomeResolvedUnitResult> getResolvedUnit(String path,
1062-
{bool sendCachedToStream = false}) {
1080+
{bool sendCachedToStream = false, bool interactive = true}) {
10631081
if (!_isAbsolutePath(path)) {
10641082
return Future.value(
10651083
InvalidPathResult(),
@@ -1091,7 +1109,11 @@ class AnalysisDriver {
10911109

10921110
// Schedule analysis.
10931111
var completer = Completer<SomeResolvedUnitResult>();
1094-
_requestedFiles.add(path, completer);
1112+
if (interactive) {
1113+
_requestedFiles.add(path, completer);
1114+
} else {
1115+
_requestedFilesNonInteractive.add(path, completer);
1116+
}
10951117
_scheduler.notify();
10961118
return completer.future;
10971119
}
@@ -1245,6 +1267,12 @@ class AnalysisDriver {
12451267
_produceErrors(path);
12461268
return;
12471269
}
1270+
1271+
// Analyze a requested - but not interactivly requested - file.
1272+
if (_requestedFilesNonInteractive.firstKey case var path?) {
1273+
_analyzeFile(path);
1274+
return;
1275+
}
12481276
}
12491277

12501278
/// Remove the file with the given [path] from the list of files to analyze.
@@ -1412,6 +1440,8 @@ class AnalysisDriver {
14121440

14131441
// getResolvedUnit()
14141442
_requestedFiles.completeAll(unitFile.path, resolvedUnit);
1443+
_requestedFilesNonInteractive.completeAll(
1444+
unitFile.path, resolvedUnit);
14151445

14161446
// getErrors()
14171447
_errorsRequestedFiles.completeAll(
@@ -1517,6 +1547,9 @@ class AnalysisDriver {
15171547
completeWithError(
15181548
_requestedFiles.remove(file.path),
15191549
);
1550+
completeWithError(
1551+
_requestedFilesNonInteractive.remove(file.path),
1552+
);
15201553
// getErrors()
15211554
completeWithError(
15221555
_errorsRequestedFiles.remove(file.path),

0 commit comments

Comments
 (0)