Skip to content

Commit

Permalink
Remove only libraries potentially affected by the changed file.
Browse files Browse the repository at this point in the history
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: flutter/flutter-intellij#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>
  • Loading branch information
scheglov authored and commit-bot@chromium.org committed Sep 25, 2021
1 parent b85488c commit 667e879
Show file tree
Hide file tree
Showing 9 changed files with 253 additions and 55 deletions.
2 changes: 1 addition & 1 deletion pkg/analysis_server/test/lsp/server_abstract.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
84 changes: 48 additions & 36 deletions pkg/analyzer/lib/src/dart/analysis/driver.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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].
///
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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);
}
}

Expand All @@ -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
Expand All @@ -500,6 +503,7 @@ class AnalysisDriver implements AnalysisDriverGeneric {
/// periodically.
@visibleForTesting
void clearLibraryContext() {
_libraryContext?.invalidAllLibraries();
_libraryContext = null;
_currentSession.clearHierarchies();
}
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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) {
Expand All @@ -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
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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() {
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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;
}
Expand Down
20 changes: 20 additions & 0 deletions pkg/analyzer/lib/src/dart/analysis/file_state.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
16 changes: 4 additions & 12 deletions pkg/analyzer/lib/src/dart/analysis/file_tracker.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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".
Expand Down Expand Up @@ -165,7 +158,6 @@ class FileTracker {
// files seems extreme.
_fsState.removeFile(path);
_pendingFiles.addAll(addedFiles);
_changeHook();
}

/// Schedule all added files for analysis.
Expand Down
5 changes: 5 additions & 0 deletions pkg/analyzer/lib/src/dart/analysis/library_context.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
6 changes: 6 additions & 0 deletions pkg/analyzer/lib/src/dart/element/element.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
17 changes: 16 additions & 1 deletion pkg/analyzer/lib/src/summary2/linked_element_factory.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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?;
Expand All @@ -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);
Expand Down Expand Up @@ -239,4 +247,11 @@ class LinkedElementFactory {

libraryElement.createLoadLibraryFunction();
}

void _invalidateLibrary(Reference? libraryReference) {
var libraryElement = libraryReference?.element;
if (libraryElement is LibraryElementImpl) {
libraryElement.isValid = false;
}
}
}
4 changes: 2 additions & 2 deletions pkg/analyzer/lib/src/summary2/reference.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading

0 comments on commit 667e879

Please sign in to comment.