Remove only libraries potentially affected by the changed file.

For a small analyzer library the time to analyze it 100 times,
each time changing that file that we are analyzing, so simulating
typing in this file, goes from 1000 ms to 100 ms; for a bigger
library 2400 vs 800 ms.

For a tiny Flutter hello_world the difference is 100x, 5800 vs 50 ms.

Bug: https://github.com/flutter/flutter-intellij/issues/5761
Change-Id: I15c43f4cd66399788ffe6032c52fb365665de7f8
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/214502
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Konstantin Shcheglov <scheglov@google.com>

pkg/analysis_server/test/lsp/server_abstract.dart

@@ -1467,7 +1467,7 @@ mixin LspAnalysisServerTestMixin implements ClientCapabilitiesHelperMixin {
               text: content)),
     );
     await sendNotificationToServer(notification);
-    await pumpEventQueue();
+    await pumpEventQueue(times: 128);
   }
 
   int positionCompare(Position p1, Position p2) {

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

@@ -222,7 +222,7 @@ class AnalysisDriver implements AnalysisDriverGeneric {
   final bool enableIndex;
 
   /// The current analysis session.
-  late AnalysisSessionImpl _currentSession;
+  late final AnalysisSessionImpl _currentSession = AnalysisSessionImpl(this);
 
   /// The current library context, consistent with the [_currentSession].
   ///
@@ -299,7 +299,6 @@ class AnalysisDriver implements AnalysisDriverGeneric {
         _sourceFactory = sourceFactory,
         _externalSummaries = externalSummaries,
         testingData = retainDataForTesting ? TestingData() : null {
-    _createNewSession();
     _onResults = _resultController.stream.asBroadcastStream();
     _testView = AnalysisDriverTestView(this);
     _createFileTracker();
@@ -461,14 +460,10 @@ class AnalysisDriver implements AnalysisDriverGeneric {
       return;
     }
     if (file_paths.isDart(resourceProvider.pathContext, path)) {
+      _priorityResults.clear();
+      _removePotentiallyAffectedLibraries(path);
       _fileTracker.addFile(path);
-      // If the file is known, it has already been read, even if it did not
-      // exist. Now we are notified that the file exists, so we need to
-      // re-read it and make sure that we invalidate signature of the files
-      // that reference it.
-      if (_fsState.knownFilePaths.contains(path)) {
-        _changeFile(path);
-      }
+      _scheduler.notify(this);
     }
   }
 
@@ -490,7 +485,15 @@ class AnalysisDriver implements AnalysisDriverGeneric {
   /// [changeFile] invocation.
   void changeFile(String path) {
     _throwIfNotAbsolutePath(path);
-    _changeFile(path);
+    if (!_fsState.hasUri(path)) {
+      return;
+    }
+    if (file_paths.isDart(resourceProvider.pathContext, path)) {
+      _priorityResults.clear();
+      _removePotentiallyAffectedLibraries(path);
+      _fileTracker.changeFile(path);
+      _scheduler.notify(this);
+    }
   }
 
   /// Clear the library context and any related data structures. Mostly we do
@@ -500,6 +503,7 @@ class AnalysisDriver implements AnalysisDriverGeneric {
   /// periodically.
   @visibleForTesting
   void clearLibraryContext() {
+    _libraryContext?.invalidAllLibraries();
     _libraryContext = null;
     _currentSession.clearHierarchies();
   }
@@ -528,9 +532,15 @@ class AnalysisDriver implements AnalysisDriverGeneric {
     if (sourceFactory != null) {
       _sourceFactory = sourceFactory;
     }
+
+    _priorityResults.clear();
+    clearLibraryContext();
+
     Iterable<String> addedFiles = _fileTracker.addedFiles;
     _createFileTracker();
     _fileTracker.addFiles(addedFiles);
+
+    _scheduler.notify(this);
   }
 
   /// Return a [Future] that completes when discovery of all files that are
@@ -1197,17 +1207,25 @@ class AnalysisDriver implements AnalysisDriverGeneric {
   /// but does not guarantee this.
   void removeFile(String path) {
     _throwIfNotAbsolutePath(path);
-    _fileTracker.removeFile(path);
-    clearLibraryContext();
-    _priorityResults.clear();
+    if (!_fsState.hasUri(path)) {
+      return;
+    }
+    if (file_paths.isDart(resourceProvider.pathContext, path)) {
+      _priorityResults.clear();
+      _removePotentiallyAffectedLibraries(path);
+      _fileTracker.removeFile(path);
+      _scheduler.notify(this);
+    }
   }
 
   /// Reset URI resolution, read again all files, build files graph, and ensure
   /// that for all added files new results are reported.
   void resetUriResolution() {
+    _priorityResults.clear();
+    clearLibraryContext();
     _fsState.resetUriResolution();
     _fileTracker.scheduleAllAddedFiles();
-    _changeHook();
+    _scheduler.notify(this);
   }
 
   void _addDeclaredVariablesToSignature(ApiSignature buffer) {
@@ -1221,22 +1239,6 @@ class AnalysisDriver implements AnalysisDriverGeneric {
     }
   }
 
-  /// Implementation for [changeFile].
-  void _changeFile(String path) {
-    _fileTracker.changeFile(path);
-    clearLibraryContext();
-    _priorityResults.clear();
-  }
-
-  /// Handles a notification from the [FileTracker] that there has been a change
-  /// of state.
-  void _changeHook() {
-    _createNewSession();
-    clearLibraryContext();
-    _priorityResults.clear();
-    _scheduler.notify(this);
-  }
-
   /// There was an exception during a file analysis, we don't know why.
   /// But it might have been caused by an inconsistency of files state, and
   /// the library context state. Reset the library context, and hope that
@@ -1483,7 +1485,7 @@ class AnalysisDriver implements AnalysisDriverGeneric {
       externalSummaries: _externalSummaries,
       fileContentCache: _fileContentCache,
     );
-    _fileTracker = FileTracker(_logger, _fsState, _changeHook);
+    _fileTracker = FileTracker(_logger, _fsState);
   }
 
   /// Return the context in which the [library] should be analyzed.
@@ -1516,11 +1518,6 @@ class AnalysisDriver implements AnalysisDriverGeneric {
     return libraryContext;
   }
 
-  /// Create a new analysis session, so invalidating the current one.
-  void _createNewSession() {
-    _currentSession = AnalysisSessionImpl(this);
-  }
-
   /// If this has not been done yet, schedule discovery of all files that are
   /// potentially available, so that they are included in [knownFiles].
   void _discoverAvailableFiles() {
@@ -1682,6 +1679,18 @@ class AnalysisDriver implements AnalysisDriverGeneric {
         'missing', errorsResult, AnalysisDriverUnitIndexBuilder());
   }
 
+  void _removePotentiallyAffectedLibraries(String path) {
+    _logger.run('Invalidate affected by $path.', () {
+      _logger.writeln('Work in $name');
+      var affected = <FileState>{};
+      _fsState.collectAffected(path, affected);
+      _logger.writeln('Remove ${affected.length} libraries.');
+      _libraryContext?.elementFactory.removeLibraries(
+        affected.map((e) => e.uriStr).toSet(),
+      );
+    });
+  }
+
   void _reportException(String path, Object exception, StackTrace stackTrace) {
     String? contextKey;
     if (exception is _ExceptionState) {
@@ -2053,6 +2062,9 @@ class AnalysisDriverTestView {
 
   FileTracker get fileTracker => driver._fileTracker;
 
+  /// TODO(scheglov) Rename this, and [libraryContext].
+  LibraryContext? get libraryContext2 => driver._libraryContext;
+
   Map<String, ResolvedUnitResult> get priorityResults {
     return driver._priorityResults;
   }

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

@@ -781,6 +781,26 @@ class FileSystemState {
   @visibleForTesting
   FileSystemStateTestView get test => _testView;
 
+  /// Collected files that transitively reference a file with the [path].
+  /// These files are potentially affected by the change.
+  void collectAffected(String path, Set<FileState> affected) {
+    final knownFiles = this.knownFiles.toList();
+
+    collectAffected(FileState file) {
+      if (affected.add(file)) {
+        for (var other in knownFiles) {
+          if (other.directReferencedFiles.contains(file)) {
+            collectAffected(other);
+          }
+        }
+      }
+    }
+
+    for (var file in _pathToFiles[path] ?? <FileState>[]) {
+      collectAffected(file);
+    }
+  }
+
   FeatureSet contextFeatureSet(
     String path,
     Uri uri,

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

@@ -16,10 +16,6 @@ import 'package:analyzer/src/dart/analysis/performance_logger.dart';
 ///
 /// Provides methods for updating the file system state in response to changes.
 class FileTracker {
-  /// Callback invoked whenever a change occurs that may require the client to
-  /// perform analysis.
-  final void Function() _changeHook;
-
   /// The logger to write performed operations and performance to.
   final PerformanceLog _logger;
 
@@ -49,7 +45,7 @@ class FileTracker {
   /// have any special relation with changed files.
   var _pendingFiles = <String>{};
 
-  FileTracker(this._logger, this._fsState, this._changeHook);
+  FileTracker(this._logger, this._fsState);
 
   /// Returns the path to exactly one that needs analysis.  Throws a
   /// [StateError] if no files need analysis.
@@ -100,27 +96,24 @@ class FileTracker {
 
   /// Adds the given [path] to the set of "added files".
   void addFile(String path) {
-    _fsState.markFileForReading(path);
     addedFiles.add(path);
-    _pendingFiles.add(path);
-    _changeHook();
+    changeFile(path);
   }
 
   /// Adds the given [paths] to the set of "added files".
   void addFiles(Iterable<String> paths) {
     addedFiles.addAll(paths);
     _pendingFiles.addAll(paths);
-    _changeHook();
   }
 
   /// Adds the given [path] to the set of "changed files".
   void changeFile(String path) {
+    _fsState.markFileForReading(path);
     _changedFiles.add(path);
+
     if (addedFiles.contains(path)) {
       _pendingChangedFiles.add(path);
     }
-    _fsState.markFileForReading(path);
-    _changeHook();
   }
 
   /// Removes the given [path] from the set of "pending files".
@@ -165,7 +158,6 @@ class FileTracker {
     // files seems extreme.
     _fsState.removeFile(path);
     _pendingFiles.addAll(addedFiles);
-    _changeHook();
   }
 
   /// Schedule all added files for analysis.

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

@@ -89,6 +89,11 @@ class LibraryContext {
     return elementFactory.libraryOfUri2('$uri');
   }
 
+  /// We are about to discard this context, mark all libraries invalid.
+  void invalidAllLibraries() {
+    elementFactory.invalidateAllLibraries();
+  }
+
   /// Load data required to access elements of the given [targetLibrary].
   void load2(FileState targetLibrary) {
     timerLoad2.start();

pkg/analyzer/lib/src/dart/element/element.dart

@@ -3723,6 +3723,12 @@ class LibraryElementImpl extends _ExistingElementImpl
   @override
   final AnalysisSession session;
 
+  /// If `true`, then this library is valid in the session.
+  ///
+  /// A library becomes invalid when one of its files, or one of its
+  /// dependencies, changes.
+  bool isValid = true;
+
   /// The language version for the library.
   LibraryLanguageVersion? _languageVersion;
 

pkg/analyzer/lib/src/summary2/linked_element_factory.dart

@@ -164,6 +164,13 @@ class LinkedElementFactory {
     return libraryReaders[uriStr] != null;
   }
 
+  /// We are about to discard this factory, mark all libraries invalid.
+  void invalidateAllLibraries() {
+    for (var libraryReference in rootReference.children) {
+      _invalidateLibrary(libraryReference);
+    }
+  }
+
   LibraryElementImpl? libraryOfUri(String uriStr) {
     var reference = rootReference.getChild(uriStr);
     return elementOfReference(reference) as LibraryElementImpl?;
@@ -189,7 +196,8 @@ class LinkedElementFactory {
     for (var uriStr in uriStrSet) {
       _exportsOfLibrary.remove(uriStr);
       libraryReaders.remove(uriStr);
-      rootReference.removeChild(uriStr);
+      var libraryReference = rootReference.removeChild(uriStr);
+      _invalidateLibrary(libraryReference);
     }
 
     analysisSession.classHierarchy.removeOfLibraries(uriStrSet);
@@ -239,4 +247,11 @@ class LinkedElementFactory {
 
     libraryElement.createLoadLibraryFunction();
   }
+
+  void _invalidateLibrary(Reference? libraryReference) {
+    var libraryElement = libraryReference?.element;
+    if (libraryElement is LibraryElementImpl) {
+      libraryElement.isValid = false;
+    }
+  }
 }

pkg/analyzer/lib/src/summary2/reference.dart

@@ -65,8 +65,8 @@ class Reference {
     return map[name] ??= Reference._(this, name);
   }
 
-  void removeChild(String name) {
-    _children?.remove(name);
+  Reference? removeChild(String name) {
+    return _children?.remove(name);
   }
 
   @override

pkg/analyzer/test/src/dart/analysis/driver_test.dart

@@ -727,6 +727,128 @@ var A = B;
     expect(driver.fsState.knownFilePaths, isNot(contains(b)));
   }
 
+  test_changeFile_potentiallyAffected_imported() async {
+    newFile('/test/lib/a.dart', content: '');
+    var b = newFile('/test/lib/b.dart', content: '''
+import 'a.dart';
+''');
+    newFile('/test/lib/c.dart', content: '''
+import 'b.dart';
+''');
+    newFile('/test/lib/d.dart', content: '''
+import 'c.dart';
+''');
+    newFile('/test/lib/e.dart', content: '');
+
+    Future<LibraryElementImpl> getLibrary(String shortName) async {
+      var uriStr = 'package:test/$shortName';
+      var result = await driver.getLibraryByUriValid(uriStr);
+      return result.element as LibraryElementImpl;
+    }
+
+    var a_element = await getLibrary('a.dart');
+    var b_element = await getLibrary('b.dart');
+    var c_element = await getLibrary('c.dart');
+    var d_element = await getLibrary('d.dart');
+    var e_element = await getLibrary('e.dart');
+
+    // We have all libraries loaded after analysis.
+    driver.assertLoadedLibraryUriSet(
+      included: [
+        'package:test/a.dart',
+        'package:test/b.dart',
+        'package:test/c.dart',
+        'package:test/d.dart',
+        'package:test/e.dart',
+      ],
+    );
+
+    // All libraries are valid.
+    expect(a_element.isValid, true);
+    expect(b_element.isValid, true);
+    expect(c_element.isValid, true);
+    expect(d_element.isValid, true);
+    expect(e_element.isValid, true);
+
+    // Change `b.dart`, also removes `c.dart` and `d.dart` that import it.
+    // But `a.dart` and `d.dart` is not affected.
+    driver.changeFile(b.path);
+    driver.assertLoadedLibraryUriSet(
+      excluded: [
+        'package:test/b.dart',
+        'package:test/c.dart',
+        'package:test/d.dart',
+      ],
+      included: [
+        'package:test/a.dart',
+        'package:test/e.dart',
+      ],
+    );
+
+    // Only `a.dart` and `e.dart` are still valid.
+    expect(a_element.isValid, true);
+    expect(b_element.isValid, false);
+    expect(c_element.isValid, false);
+    expect(d_element.isValid, false);
+    expect(e_element.isValid, true);
+  }
+
+  test_changeFile_potentiallyAffected_part() async {
+    var a = newFile('/test/lib/a.dart', content: '''
+part of 'b.dart';
+''');
+    newFile('/test/lib/b.dart', content: '''
+part 'a.dart';
+''');
+    newFile('/test/lib/c.dart', content: '''
+import 'b.dart';
+''');
+    newFile('/test/lib/d.dart', content: '');
+
+    Future<LibraryElementImpl> getLibrary(String shortName) async {
+      var uriStr = 'package:test/$shortName';
+      var result = await driver.getLibraryByUriValid(uriStr);
+      return result.element as LibraryElementImpl;
+    }
+
+    var b_element = await getLibrary('b.dart');
+    var c_element = await getLibrary('c.dart');
+    var d_element = await getLibrary('d.dart');
+
+    // We have all libraries loaded after analysis.
+    driver.assertLoadedLibraryUriSet(
+      included: [
+        'package:test/b.dart',
+        'package:test/c.dart',
+        'package:test/d.dart',
+      ],
+    );
+
+    // All libraries are valid.
+    expect(b_element.isValid, true);
+    expect(c_element.isValid, true);
+    expect(d_element.isValid, true);
+
+    // Change `a.dart`, remove `b.dart` that part it.
+    // Removes `c.dart` that imports `b.dart`.
+    // But `d.dart` is not affected.
+    driver.changeFile(a.path);
+    driver.assertLoadedLibraryUriSet(
+      excluded: [
+        'package:test/b.dart',
+        'package:test/c.dart',
+      ],
+      included: [
+        'package:test/d.dart',
+      ],
+    );
+
+    // Only `d.dart` is still valid.
+    expect(b_element.isValid, false);
+    expect(c_element.isValid, false);
+    expect(d_element.isValid, true);
+  }
+
   test_changeFile_selfConsistent() async {
     var a = convertPath('/test/lib/a.dart');
     var b = convertPath('/test/lib/b.dart');
@@ -987,7 +1109,7 @@ const x = 1;
   }
 
   test_currentSession() async {
-    var a = convertPath('/a.dart');
+    var a = convertPath('/test/lib/a.dart');
 
     newFile(a, content: 'var V = 1;');
     await driver.getResultValid(a);
@@ -1002,8 +1124,9 @@ const x = 1;
     var session2 = driver.currentSession;
     expect(session2, isNotNull);
 
-    // We get a new session.
-    expect(session2, isNot(session1));
+    // We don't discard the session anymore.
+    // So, the session is always the same.
+    expect(session2, same(session1));
   }
 
   test_discoverAvailableFiles_packages() async {
@@ -3349,7 +3472,32 @@ extension on AnalysisDriver {
     return getFileSync2(path) as FileResult;
   }
 
+  Set<String> get loadedLibraryUriSet {
+    var elementFactory = this.test.libraryContext2!.elementFactory;
+    var libraryReferences = elementFactory.rootReference.children;
+    return libraryReferences.map((e) => e.name).toSet();
+  }
+
+  void assertLoadedLibraryUriSet({
+    Iterable<String>? included,
+    Iterable<String>? excluded,
+  }) {
+    var uriSet = loadedLibraryUriSet;
+    if (included != null) {
+      expect(uriSet, containsAll(included));
+    }
+    if (excluded != null) {
+      for (var excludedUri in excluded) {
+        expect(uriSet, isNot(contains(excludedUri)));
+      }
+    }
+  }
+
   Future<ResolvedUnitResult> getResultValid(String path) async {
     return await getResult2(path) as ResolvedUnitResult;
   }
+
+  Future<LibraryElementResult> getLibraryByUriValid(String uriStr) async {
+    return await getLibraryByUri2(uriStr) as LibraryElementResult;
+  }
 }