If possible, reuse the last completion result.

Also rename "character" to "column".

[email protected], [email protected]

Change-Id: I6167de6e1fd5aede2a619603635e96f16bfbcc8d
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/144352
Reviewed-by: Brian Wilkerson <[email protected]>
Commit-Queue: Konstantin Shcheglov <[email protected]>

Author: scheglov

pkg/analysis_server/lib/src/cider/completion.dart

@@ -10,7 +10,6 @@ import 'package:analysis_server/src/services/completion/completion_performance.d
 import 'package:analysis_server/src/services/completion/dart/completion_manager.dart';
 import 'package:analysis_server/src/services/completion/dart/local_library_contributor.dart';
 import 'package:analyzer/dart/element/element.dart' show LibraryElement;
-import 'package:analyzer/source/line_info.dart';
 import 'package:analyzer/src/dart/analysis/performance_logger.dart';
 import 'package:analyzer/src/dart/micro/resolve_file.dart';
 import 'package:analyzer/src/dartdoc/dartdoc_directive_info.dart';
@@ -23,6 +22,7 @@ import 'package:meta/meta.dart';
 /// example types and elements.
 class CiderCompletionCache {
   final Map<String, _CiderImportedLibrarySuggestions> _importedLibraries = {};
+  _LastCompletionResult _lastResult;
 }
 
 class CiderCompletionComputer {
@@ -31,22 +31,26 @@ class CiderCompletionComputer {
   final FileResolver _fileResolver;
 
   DartCompletionRequestImpl _dartCompletionRequest;
-  final List<String> _computedImportedLibraries = [];
+
+  /// Paths of imported libraries for which suggestions were (re)computed
+  /// during processing of this request. Does not include libraries that were
+  /// processed during previous requests, and reused from the cache now.
+  @visibleForTesting
+  final List<String> computedImportedLibraries = [];
 
   CiderCompletionComputer(this._logger, this._cache, this._fileResolver);
 
   @deprecated
   Future<List<CompletionSuggestion>> compute(String path, int offset) async {
-    var file = _fileResolver.resourceProvider.getFile(path);
-    var content = file.readAsStringSync();
+    var fileContext = _fileResolver.getFileContext(path);
+    var file = fileContext.file;
 
-    var lineInfo = LineInfo.fromContent(content);
-    var location = lineInfo.getLocation(offset);
+    var location = file.lineInfo.getLocation(offset);
 
     var result = await compute2(
       path: path,
       line: location.lineNumber - 1,
-      character: location.columnNumber - 1,
+      column: location.columnNumber - 1,
     );
 
     return result.suggestions;
@@ -58,19 +62,36 @@ class CiderCompletionComputer {
   ///
   /// The content of the file has already been updated.
   ///
-  /// The [line] and [character] are zero based.
+  /// The [line] and [column] are zero based.
   Future<CiderCompletionResult> compute2({
     @required String path,
     @required int line,
-    @required int character,
+    @required int column,
   }) async {
-    var file = _fileResolver.resourceProvider.getFile(path);
-    var content = file.readAsStringSync();
+    var fileContext = _logger.run('Get file $path', () {
+      return _fileResolver.getFileContext(path);
+    });
 
-    var resolvedUnit = _fileResolver.resolve(path);
+    var file = fileContext.file;
 
-    var lineInfo = LineInfo.fromContent(content);
-    var offset = lineInfo.getOffsetOfLine(line) + character;
+    var resolvedSignature = _logger.run('Get signature', () {
+      return file.resolvedSignature;
+    });
+
+    var lineInfo = file.lineInfo;
+    var offset = lineInfo.getOffsetOfLine(line) + column;
+
+    // If the same file, in the same state as the last time, reuse the result.
+    var lastResult = _cache._lastResult;
+    if (lastResult != null &&
+        lastResult.path == path &&
+        lastResult.signature == resolvedSignature &&
+        lastResult.offset == offset) {
+      _logger.writeln('Use the last completion result.');
+      return lastResult.result;
+    }
+
+    var resolvedUnit = _fileResolver.resolve(path);
 
     var completionRequest = CompletionRequestImpl(
       resolvedUnit,
@@ -100,18 +121,22 @@ class CiderCompletionComputer {
       dartdocDirectiveInfo,
     );
 
-    _logger.run('Add imported suggestions', () {
-      suggestions.addAll(
-        _importedLibrariesSuggestions(
-          resolvedUnit.libraryElement,
-        ),
-      );
-    });
+    if (_dartCompletionRequest.includeIdentifiers) {
+      _logger.run('Add imported suggestions', () {
+        suggestions.addAll(
+          _importedLibrariesSuggestions(
+            resolvedUnit.libraryElement,
+          ),
+        );
+      });
+    }
 
-    return CiderCompletionResult._(
-      suggestions,
-      _computedImportedLibraries,
-    );
+    var result = CiderCompletionResult._(suggestions);
+
+    _cache._lastResult =
+        _LastCompletionResult(path, resolvedSignature, offset, result);
+
+    return result;
   }
 
   /// Return suggestions from libraries imported into the [target].
@@ -139,7 +164,7 @@ class CiderCompletionComputer {
 
     var cacheEntry = _cache._importedLibraries[path];
     if (cacheEntry == null || cacheEntry.signature != signature) {
-      _computedImportedLibraries.add(path);
+      computedImportedLibraries.add(path);
       var suggestions = _librarySuggestions(element);
       cacheEntry = _CiderImportedLibrarySuggestions(
         signature,
@@ -165,16 +190,7 @@ class CiderCompletionComputer {
 class CiderCompletionResult {
   final List<CompletionSuggestion> suggestions;
 
-  /// Paths of imported libraries for which suggestions were (re)computed
-  /// during processing of this request. Does not include libraries that were
-  /// processed during previous requests, and reused from the cache now.
-  @visibleForTesting
-  final List<String> computedImportedLibraries;
-
-  CiderCompletionResult._(
-    this.suggestions,
-    this.computedImportedLibraries,
-  );
+  CiderCompletionResult._(this.suggestions);
 }
 
 class _CiderImportedLibrarySuggestions {
@@ -183,3 +199,12 @@ class _CiderImportedLibrarySuggestions {
 
   _CiderImportedLibrarySuggestions(this.signature, this.suggestions);
 }
+
+class _LastCompletionResult {
+  final String path;
+  final String signature;
+  final int offset;
+  final CiderCompletionResult result;
+
+  _LastCompletionResult(this.path, this.signature, this.offset, this.result);
+}

pkg/analysis_server/test/src/cider/completion_test.dart

@@ -22,6 +22,7 @@ void main() {
 class CiderCompletionComputerTest extends CiderServiceTest {
   final CiderCompletionCache _completionCache = CiderCompletionCache();
 
+  CiderCompletionComputer _computer;
   CiderCompletionResult _completionResult;
   List<CompletionSuggestion> _suggestions;
 
@@ -78,6 +79,22 @@ main(int b) {
     _assertNoClass(text: 'Random');
   }
 
+  Future<void> test_compute2_sameSignature_sameResult() async {
+    _createFileResolver();
+    await _compute2(r'''
+var a = ^;
+''');
+    var lastResult = _completionResult;
+
+    // Ask for completion using new resolver and computer.
+    // But the file signature is the same, so the same result.
+    _createFileResolver();
+    await _compute2(r'''
+var a = ^;
+''');
+    expect(_completionResult, same(lastResult));
+  }
+
   Future<void> test_compute2_updateImportedLibrary() async {
     var corePath = convertPath('/sdk/lib/core/core.dart');
 
@@ -142,7 +159,7 @@ var a = ^;
   void _assertComputedImportedLibraries(List<String> expected) {
     expected = expected.map(convertPath).toList();
     expect(
-      _completionResult.computedImportedLibraries,
+      _computer.computedImportedLibraries,
       unorderedEquals(expected),
     );
   }
@@ -174,7 +191,7 @@ var a = ^;
     _completionResult = await _newComputer().compute2(
       path: testPath,
       line: context.line,
-      character: context.character,
+      column: context.character,
     );
     _suggestions = _completionResult.suggestions;
   }
@@ -202,7 +219,11 @@ var a = ^;
   }
 
   CiderCompletionComputer _newComputer() {
-    return CiderCompletionComputer(logger, _completionCache, fileResolver);
+    return _computer = CiderCompletionComputer(
+      logger,
+      _completionCache,
+      fileResolver,
+    );
   }
 
   _CompletionContext _updateFile(String content) {

pkg/analyzer/lib/src/dart/micro/library_graph.dart

@@ -87,9 +87,32 @@ class FileState {
 
   LineInfo get lineInfo => LineInfo(unlinked2.lineStarts);
 
+  /// The resolved signature of the file, that depends on the [libraryCycle]
+  /// signature, and the content of the file.
+  String get resolvedSignature {
+    var signatureBuilder = ApiSignature();
+    signatureBuilder.addString(path);
+    signatureBuilder.addBytes(libraryCycle.signature);
+
+    var content = getContent();
+    signatureBuilder.addString(content);
+
+    return signatureBuilder.toHex();
+  }
+
   /// Return the [uri] string.
   String get uriStr => uri.toString();
 
+  /// Return the content of the file, the empty string if cannot be read.
+  String getContent() {
+    try {
+      var resource = _fsState._resourceProvider.getFile(path);
+      return resource.readAsStringSync();
+    } catch (_) {
+      return '';
+    }
+  }
+
   void internal_setLibraryCycle(LibraryCycle cycle, String signature) {
     _libraryCycle = cycle;
   }
@@ -148,12 +171,7 @@ class FileState {
       }
 
       if (bytes == null || bytes.isEmpty) {
-        String content;
-        try {
-          content = _fsState._resourceProvider.getFile(path).readAsStringSync();
-        } catch (_) {
-          content = '';
-        }
+        var content = getContent();
         var unit = parse(AnalysisErrorListener.NULL_LISTENER, content);
         _fsState._logger.run('Create unlinked for $path', () {
           var unlinkedBuilder = serializeAstCiderUnlinked(_digest, unit);

pkg/analyzer/lib/src/dart/micro/resolve_file.dart

@@ -41,6 +41,13 @@ import 'package:collection/collection.dart';
 import 'package:meta/meta.dart';
 import 'package:yaml/yaml.dart';
 
+class FileContext {
+  final AnalysisOptionsImpl analysisOptions;
+  final FileState file;
+
+  FileContext(this.analysisOptions, this.file);
+}
+
 class FileResolver {
   final PerformanceLog logger;
   final ResourceProvider resourceProvider;
@@ -83,7 +90,7 @@ class FileResolver {
 
     return logger.run('Get errors for $path', () {
       var fileContext = logger.run('Get file $path', () {
-        return _createFileContext(path);
+        return getFileContext(path);
       });
       var file = fileContext.file;
 
@@ -127,10 +134,19 @@ class FileResolver {
     });
   }
 
+  FileContext getFileContext(String path) {
+    var analysisOptions = _getAnalysisOptions(path);
+
+    _createContext(analysisOptions);
+
+    var file = fsState.getFileForPath(path);
+    return FileContext(analysisOptions, file);
+  }
+
   String getLibraryLinkedSignature(String path) {
     _throwIfNotAbsoluteNormalizedPath(path);
 
-    var fileContext = _createFileContext(path);
+    var fileContext = getFileContext(path);
     var file = fileContext.file;
     return file.libraryCycle.signatureStr;
   }
@@ -140,7 +156,7 @@ class FileResolver {
 
     return logger.run('Resolve $path', () {
       var fileContext = logger.run('Get file $path', () {
-        return _createFileContext(path);
+        return getFileContext(path);
       });
       var file = fileContext.file;
 
@@ -234,22 +250,20 @@ class FileResolver {
     }
 
     if (analysisContext == null) {
-      logger.run('Create AnalysisContext', () {
-        var root = ContextRootImpl(
-          resourceProvider,
-          resourceProvider.getFolder(workspace.root),
-        );
+      var root = ContextRootImpl(
+        resourceProvider,
+        resourceProvider.getFolder(workspace.root),
+      );
 
-        analysisContext = MicroAnalysisContextImpl(
-          this,
-          root,
-          analysisOptions,
-          DeclaredVariables(),
-          sourceFactory,
-          resourceProvider,
-          workspace: workspace,
-        );
-      });
+      analysisContext = MicroAnalysisContextImpl(
+        this,
+        root,
+        analysisOptions,
+        DeclaredVariables(),
+        sourceFactory,
+        resourceProvider,
+        workspace: workspace,
+      );
     }
 
     if (libraryContext == null) {
@@ -265,15 +279,6 @@ class FileResolver {
     }
   }
 
-  _FileContext _createFileContext(String path) {
-    var analysisOptions = _getAnalysisOptions(path);
-
-    _createContext(analysisOptions);
-
-    var file = fsState.getFileForPath(path);
-    return _FileContext(analysisOptions, file);
-  }
-
   File _findOptionsFile(Folder folder) {
     while (folder != null) {
       File packagesFile =
@@ -359,13 +364,6 @@ class FileResolverTestView {
   }
 }
 
-class _FileContext {
-  final AnalysisOptionsImpl analysisOptions;
-  final FileState file;
-
-  _FileContext(this.analysisOptions, this.file);
-}
-
 class _LibraryContext {
   final PerformanceLog logger;
   final ResourceProvider resourceProvider;