[analyzer server] Legacy protocol debounces getFixes and getAsssits requests too; benchmark

jensjoha · Commit Queue · commit 3873af28e965 · 2025-03-11T00:22:15.000-07:00
In reports of the analyzer being slow we've seen `edit.getFixes` causing a long queue because they take longer to execute than the wait before the next one comes in. While we haven't been able to reproduce that, this CL adds a benchmark that fires *a lot* of both `edit.getFixes` (seen in reports from users) and `edit.getAssists` (which seems, locally at least, to happen every time the cursor moves), and debounces them, changing the benchmark results from ``` 4 files / CodeType.ImportCycle: Initial analysis: 1.322030 Completion after change: 3.096128 4 files / CodeType.ImportChain: Initial analysis: 1.361750 Completion after change: 3.500849 4 files / CodeType.ImportExportCycle: Initial analysis: 1.349346 Completion after change: 3.065497 4 files / CodeType.ImportExportChain: Initial analysis: 1.367151 Completion after change: 3.246891 4 files / CodeType.ImportCycleExportChain: Initial analysis: 1.360573 Completion after change: 3.393901 ``` to ``` 4 files / CodeType.ImportCycle: Initial analysis: 1.322070 Completion after change: 0.546532 4 files / CodeType.ImportChain: Initial analysis: 1.410870 Completion after change: 0.649789 4 files / CodeType.ImportExportCycle: Initial analysis: 1.349923 Completion after change: 0.741040 4 files / CodeType.ImportExportChain: Initial analysis: 1.360396 Completion after change: 0.638332 4 files / CodeType.ImportCycleExportChain: Initial analysis: 1.354682 Completion after change: 0.658086 ``` Change-Id: Icb0423133726e02e08e204b1c59209264889f8a6 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/413682 Commit-Queue: Jens Johansen <jensj@google.com> Reviewed-by: Phil Quitslund <pquitslund@google.com>
diff --git a/pkg/analysis_server/lib/src/server/debounce_requests.dart b/pkg/analysis_server/lib/src/server/debounce_requests.dart
@@ -12,11 +12,17 @@ import 'package:analysis_server/src/channel/channel.dart';
 /// Return the stream of requests that is filtered to exclude requests for
 /// which the client does not need actual responses.
 ///
-/// If there is one completion request, and then another completion request,
-/// then most probably the user continued typing, and there is no need to
-/// compute results for the first request. But we will have to respond, an
-/// empty response is enough.
-/// The same goes for hover requests.
+/// If there is a request (for example for completion), and then another request
+/// of the same kind, then most probably the user continued typing, and there is
+/// no need to compute results for the first request. But we will have to
+/// respond, an empty response is enough.
+///
+/// Debounced requests include:
+///
+/// * `getAssists`
+/// * `getCompletions`
+/// * `getFixes`
+/// * `getHover`
 ///
 /// Discarded requests are reported into [discardedRequests].
 Stream<RequestOrResponse> debounceRequests(
@@ -60,6 +66,8 @@ class _DebounceRequests {
     var reversed = <RequestOrResponse>[];
     var abortCompletionRequests = false;
     var abortHoverRequests = false;
+    var abortAssistsRequests = false;
+    var abortFixesRequests = false;
     for (var requestOrResponse in requests.reversed) {
       if (requestOrResponse is Request) {
         if (requestOrResponse.method == ANALYSIS_REQUEST_UPDATE_CONTENT) {
@@ -102,6 +110,36 @@ class _DebounceRequests {
           } else {
             abortHoverRequests = true;
           }
+        } else if (requestOrResponse.method == EDIT_REQUEST_GET_ASSISTS) {
+          if (abortAssistsRequests) {
+            discardedRequests.add(requestOrResponse);
+            channel.sendResponse(
+              EditGetAssistsResult([]).toResponse(
+                requestOrResponse.id,
+                // We can use a null converter here because we're not sending
+                // any path.
+                clientUriConverter: null,
+              ),
+            );
+            continue;
+          } else {
+            abortAssistsRequests = true;
+          }
+        } else if (requestOrResponse.method == EDIT_REQUEST_GET_FIXES) {
+          if (abortFixesRequests) {
+            discardedRequests.add(requestOrResponse);
+            channel.sendResponse(
+              EditGetFixesResult([]).toResponse(
+                requestOrResponse.id,
+                // We can use a null converter here because we're not sending
+                // any path.
+                clientUriConverter: null,
+              ),
+            );
+            continue;
+          } else {
+            abortFixesRequests = true;
+          }
         }
       }
       reversed.add(requestOrResponse);
diff --git a/pkg/analysis_server/tool/benchmark_tools/big_chain_benchmark/legacy_many_get_fixes_and_get_assists_requests.dart b/pkg/analysis_server/tool/benchmark_tools/big_chain_benchmark/legacy_many_get_fixes_and_get_assists_requests.dart
@@ -0,0 +1,105 @@
+// Copyright (c) 2025, the Dart project authors. Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+
+import '../language_server_benchmark.dart';
+import '../legacy_messages.dart';
+import 'utils.dart';
+
+/// In reports of the analyzer being slow we've seen `edit.getFixes` causing a
+/// long queue because they take longer to execute than the wait before the next
+/// one comes in.
+/// While we haven't been able to reproduce that we can surely fire a whole lot
+/// of them very fast, being able to meassure that we "debounce" them is we get
+/// too many at once.
+/// Here we'll fire both `edit.getFixes` (seen in reports from users)
+/// and `edit.getAssists` (which seems, locally at least, to happen every time
+/// the cursor moves).
+Future<void> main() async {
+  await runHelper(
+    LegacyManyGetFixesAndGetAssisstRequestsBenchmark.new,
+    runAsLsp: false,
+    // The number of files doesn't seem to be important on this one.
+    numberOfFileOptions: [4],
+  );
+}
+
+class LegacyManyGetFixesAndGetAssisstRequestsBenchmark
+    extends DartLanguageServerBenchmark {
+  @override
+  final Uri rootUri;
+  @override
+  final Uri cacheFolder;
+
+  final RunDetails runDetails;
+
+  LegacyManyGetFixesAndGetAssisstRequestsBenchmark(
+    this.rootUri,
+    this.cacheFolder,
+    this.runDetails,
+  ) : super(useLspProtocol: false);
+
+  @override
+  LaunchFrom get launchFrom => LaunchFrom.Dart;
+
+  @override
+  Future<void> afterInitialization() async {
+    await send(
+      LegacyMessages.setPriorityFiles(largestIdSeen + 1, [
+        runDetails.mainFile.uri,
+        runDetails.orderedFileCopies.first.uri,
+      ]),
+    );
+
+    // This is probably not realistic, but not being able to reproduce fewer
+    // requests, each taking longer, for now this is better than nothing.
+    for (var i = 0; i < 2000; i++) {
+      await send(
+        LegacyMessages.getFixes(
+          largestIdSeen + 1,
+          runDetails.orderedFileCopies.first.uri,
+          i,
+        ),
+      );
+      await send(
+        LegacyMessages.getAssists(
+          largestIdSeen + 1,
+          runDetails.orderedFileCopies.first.uri,
+          i,
+        ),
+      );
+    }
+
+    // Type 'ge' in the empty line in main.
+    await send(
+      LegacyMessages.updateContent(
+        largestIdSeen + 1,
+        runDetails.mainFile.uri,
+        runDetails.mainFileTypingContent,
+      ),
+    );
+
+    // Ask for completion
+    Future<Map<String, dynamic>> completionFuture =
+        (await send(
+          LegacyMessages.getSuggestions2(
+            largestIdSeen + 1,
+            runDetails.mainFile.uri,
+            runDetails.typingAtOffset,
+          ),
+        ))!.completer.future;
+
+    Stopwatch stopwatch = Stopwatch()..start();
+    var completionResponse = await completionFuture;
+    List<dynamic> completionItems =
+        completionResponse['result']['suggestions'] as List;
+    var completionAfterChange = stopwatch.elapsed;
+    durationInfo.add(
+      DurationInfo('Completion after change', completionAfterChange),
+    );
+    print(
+      'Got ${completionItems.length} completion items '
+      'in $completionAfterChange',
+    );
+  }
+}
diff --git a/pkg/analysis_server/tool/benchmark_tools/big_chain_benchmark/legacy_many_hover_requests.dart b/pkg/analysis_server/tool/benchmark_tools/big_chain_benchmark/legacy_many_hover_requests.dart
@@ -39,19 +39,6 @@ class LegacyManyHoverRequestsBenchmark extends DartLanguageServerBenchmark {
 
   @override
   Future<void> afterInitialization() async {
-    var mainFileLines = runDetails.mainFile.content.split('\n');
-    if (!mainFileLines[2].startsWith('void main')) throw 'unexpected file data';
-    if (mainFileLines[3].trim().isNotEmpty) throw 'unexpected file data';
-    mainFileLines[3] = '  ge';
-    var newMainFileContent = StringBuffer();
-    for (int i = 0; i <= 3; i++) {
-      newMainFileContent.writeln(mainFileLines[i]);
-    }
-    var geOffset = newMainFileContent.length - 1;
-    for (int i = 4; i < mainFileLines.length; i++) {
-      newMainFileContent.writeln(mainFileLines[i]);
-    }
-
     await send(
       LegacyMessages.setPriorityFiles(largestIdSeen + 1, [
         runDetails.mainFile.uri,
@@ -77,7 +64,7 @@ class LegacyManyHoverRequestsBenchmark extends DartLanguageServerBenchmark {
       LegacyMessages.updateContent(
         largestIdSeen + 1,
         runDetails.mainFile.uri,
-        newMainFileContent.toString(),
+        runDetails.mainFileTypingContent,
       ),
     );
 
@@ -87,7 +74,7 @@ class LegacyManyHoverRequestsBenchmark extends DartLanguageServerBenchmark {
           LegacyMessages.getSuggestions2(
             largestIdSeen + 1,
             runDetails.mainFile.uri,
-            geOffset,
+            runDetails.typingAtOffset,
           ),
         ))!.completer.future;
 
diff --git a/pkg/analysis_server/tool/benchmark_tools/big_chain_benchmark/utils.dart b/pkg/analysis_server/tool/benchmark_tools/big_chain_benchmark/utils.dart
@@ -84,14 +84,25 @@ String get$i() {
 import '${getFilenameFor(1)}';
 
 void main(List<String> arguments) {
-  
+
 }
 """;
   File.fromUri(mainFileUri).writeAsStringSync(mainFileContent);
+  var typing = '  ge';
+  var mainFileTypingContent = """
+import '${getFilenameFor(1)}';
+
+void main(List<String> arguments) {
+$typing
+}
+""";
+  var typingAtOffset = mainFileTypingContent.indexOf(typing) + typing.length;
 
   return RunDetails(
     libDirUri: libDirUri,
     mainFile: FileContentPair(mainFileUri, mainFileContent),
+    mainFileTypingContent: mainFileTypingContent,
+    typingAtOffset: typingAtOffset,
     orderedFileCopies: orderedFileCopies,
     numFiles: numFiles,
   );
@@ -178,12 +189,16 @@ class FileContentPair {
 class RunDetails {
   final Uri libDirUri;
   final FileContentPair mainFile;
+  final String mainFileTypingContent;
+  final int typingAtOffset;
   final List<FileContentPair> orderedFileCopies;
   final int numFiles;
 
   RunDetails({
     required this.libDirUri,
     required this.mainFile,
+    required this.mainFileTypingContent,
+    required this.typingAtOffset,
     required this.orderedFileCopies,
     required this.numFiles,
   });
diff --git a/pkg/analysis_server/tool/benchmark_tools/language_server_benchmark.dart b/pkg/analysis_server/tool/benchmark_tools/language_server_benchmark.dart
@@ -85,6 +85,9 @@ abstract class DartLanguageServerBenchmark {
         print('Sending message with id $possibleId');
       }
     }
+    if (verbosity > 3) {
+      print('Sending message: $jsonEncodedBody');
+    }
 
     if (_lsp) {
       // Header is always ascii, body is always utf8!
@@ -295,6 +298,16 @@ abstract class DartLanguageServerBenchmark {
         String messageString = utf8.decode(_buffer);
         _buffer.clear();
         _headerContentLength = null;
+
+        if (messageString.startsWith('The Dart VM service')) {
+          print('\n\n$messageString\n\n');
+          continue;
+        }
+        if (messageString.startsWith('The Dart DevTools')) {
+          print('\n\n$messageString\n\n');
+          continue;
+        }
+
         Map<String, dynamic> message =
             json.decode(messageString) as Map<String, dynamic>;
 
diff --git a/pkg/analysis_server/tool/benchmark_tools/legacy_messages.dart b/pkg/analysis_server/tool/benchmark_tools/legacy_messages.dart
@@ -9,6 +9,29 @@ String _uriToStringWithoutEndingSlash(Uri uri) {
 }
 
 class LegacyMessages {
+  static Map<String, dynamic> getAssists(
+    int id,
+    Uri file,
+    int offset, {
+    int length = 0,
+  }) {
+    return {
+      'id': '$id',
+      'method': 'edit.getAssists',
+      'params': {'file': '$file', 'offset': offset, 'length': length},
+      'clientRequestTime': DateTime.now().millisecondsSinceEpoch,
+    };
+  }
+
+  static Map<String, dynamic> getFixes(int id, Uri file, int offset) {
+    return {
+      'id': '$id',
+      'method': 'edit.getFixes',
+      'params': {'file': '$file', 'offset': offset},
+      'clientRequestTime': DateTime.now().millisecondsSinceEpoch,
+    };
+  }
+
   static Map<String, dynamic> getHover(int id, Uri file, int offset) {
     return {
       'id': '$id',
