Skip to content

Commit

Permalink
Use OwnedFiles to track which files are owned by which driver.
Browse files Browse the repository at this point in the history
So, we don't have to recompute it at every symbol search.
For Flutter repository, searching for 'ButtonStyle' is 40% faster.

SHA: 47fe150
Description: before
  [mean: 56.4][stdDev: 1.471][min: 54.0][max: 61.0]

Description: own files
  [mean: 35.0][stdDev: 1.308][min: 33.0][max: 38.0]
Change-Id: Id497bf4117bdfafeeccd04601fd0198e22624c0d
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/305843
Reviewed-by: Samuel Rawlins <[email protected]>
Reviewed-by: Brian Wilkerson <[email protected]>
Commit-Queue: Konstantin Shcheglov <[email protected]>
  • Loading branch information
scheglov authored and Commit Queue committed May 26, 2023
1 parent 44b5ca7 commit 7d13871
Show file tree
Hide file tree
Showing 13 changed files with 107 additions and 25 deletions.
4 changes: 4 additions & 0 deletions pkg/analysis_server/lib/src/analysis_server.dart
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ import 'package:analyzer/file_system/physical_file_system.dart';
import 'package:analyzer/instrumentation/instrumentation.dart';
import 'package:analyzer/src/dart/analysis/byte_store.dart';
import 'package:analyzer/src/dart/analysis/driver.dart' as analysis;
import 'package:analyzer/src/dart/analysis/driver.dart';
import 'package:analyzer/src/dart/analysis/file_byte_store.dart'
show EvictingFileByteStore;
import 'package:analyzer/src/dart/analysis/file_content_cache.dart';
Expand Down Expand Up @@ -296,6 +297,9 @@ abstract class AnalysisServer {
/// Returns `null` is the client does not support it.
OpenUriNotificationSender? get openUriNotificationSender;

/// Returns owners of files.
OwnedFiles get ownedFiles => contextManager.ownedFiles;

/// Whether or not the client supports showMessageRequest to show the user
/// a message and allow them to respond by clicking buttons.
bool get supportsShowMessageRequest;
Expand Down
8 changes: 8 additions & 0 deletions pkg/analysis_server/lib/src/context_manager.dart
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,9 @@ abstract class ContextManager {
/// to [setRoots].
List<String> get includedPaths;

/// Returns owners of files.
OwnedFiles get ownedFiles;

/// Return the existing analysis context that should be used to analyze the
/// given [path], or `null` if the [path] is not analyzed in any of the
/// created analysis contexts.
Expand Down Expand Up @@ -251,6 +254,11 @@ class ContextManagerImpl implements ContextManager {
List<AnalysisContext> get analysisContexts =>
_collection?.contexts.cast<AnalysisContext>() ?? const [];

@override
OwnedFiles get ownedFiles {
return _collection?.ownedFiles ?? OwnedFiles();
}

@override
DriverBasedAnalysisContext? getContextFor(String path) {
try {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ class SearchGetElementDeclarationsHandler extends LegacyHandler {
params.pattern ?? '',
params.maxResults,
onlyForFile: params.file,
ownedFiles: server.ownedFiles,
performance: performance,
).compute();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ class WorkspaceSymbolHandler
query,
remainingResults,
onlyAnalyzed: searchOnlyAnalyzed,
ownedFiles: server.ownedFiles,
performance: performance,
).compute(token);
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,9 @@ class AnalysisContextCollectionImpl implements AnalysisContextCollection {
/// The instance of the macro kernel builder.
final MacroKernelBuilder macroKernelBuilder = MacroKernelBuilder();

/// The shared container into which drivers record files ownership.
final OwnedFiles ownedFiles = OwnedFiles();

/// The list of analysis contexts.
@override
final List<DriverBasedAnalysisContext> contexts = [];
Expand All @@ -59,13 +62,12 @@ class AnalysisContextCollectionImpl implements AnalysisContextCollection {
UnlinkedUnitStore? unlinkedUnitStore,
@Deprecated('Use updateAnalysisOptions2, which must be a function that '
'accepts a second parameter')
void Function(AnalysisOptionsImpl)? updateAnalysisOptions,
void Function(AnalysisOptionsImpl)? updateAnalysisOptions,
void Function({
required AnalysisOptionsImpl analysisOptions,
required ContextRoot contextRoot,
required DartSdk sdk,
})?
updateAnalysisOptions2,
})? updateAnalysisOptions2,
}) : resourceProvider =
resourceProvider ?? PhysicalResourceProvider.INSTANCE {
sdkPath ??= getSdkPath();
Expand Down Expand Up @@ -111,6 +113,7 @@ class AnalysisContextCollectionImpl implements AnalysisContextCollection {
unlinkedUnitStore: unlinkedUnitStore ?? UnlinkedUnitStoreImpl(),
macroKernelBuilder: macroKernelBuilder,
macroExecutor: macroExecutor,
ownedFiles: ownedFiles,
);
contexts.add(context);
}
Expand Down
13 changes: 9 additions & 4 deletions pkg/analyzer/lib/src/dart/analysis/context_builder.dart
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,11 @@ import 'package:analyzer/src/context/packages.dart';
import 'package:analyzer/src/dart/analysis/byte_store.dart'
show ByteStore, MemoryByteStore;
import 'package:analyzer/src/dart/analysis/driver.dart'
show AnalysisDriver, AnalysisDriverScheduler, AnalysisDriverTestView;
show
AnalysisDriver,
AnalysisDriverScheduler,
AnalysisDriverTestView,
OwnedFiles;
import 'package:analyzer/src/dart/analysis/driver_based_analysis_context.dart';
import 'package:analyzer/src/dart/analysis/file_content_cache.dart';
import 'package:analyzer/src/dart/analysis/performance_logger.dart'
Expand Down Expand Up @@ -61,17 +65,17 @@ class ContextBuilderImpl implements ContextBuilder {
String? sdkPath,
String? sdkSummaryPath,
@Deprecated('Use updateAnalysisOptions2')
void Function(AnalysisOptionsImpl)? updateAnalysisOptions,
void Function(AnalysisOptionsImpl)? updateAnalysisOptions,
void Function({
required AnalysisOptionsImpl analysisOptions,
required ContextRoot contextRoot,
required DartSdk sdk,
})?
updateAnalysisOptions2,
})? updateAnalysisOptions2,
FileContentCache? fileContentCache,
UnlinkedUnitStore? unlinkedUnitStore,
MacroKernelBuilder? macroKernelBuilder,
macro.MultiMacroExecutor? macroExecutor,
OwnedFiles? ownedFiles,
}) {
// TODO(scheglov) Remove this, and make `sdkPath` required.
sdkPath ??= getSdkPath();
Expand Down Expand Up @@ -149,6 +153,7 @@ class ContextBuilderImpl implements ContextBuilder {
macroExecutor: macroExecutor,
declaredVariables: declaredVariables,
testView: retainDataForTesting ? AnalysisDriverTestView() : null,
ownedFiles: ownedFiles,
);

// AnalysisDriver reports results into streams.
Expand Down
40 changes: 40 additions & 0 deletions pkg/analyzer/lib/src/dart/analysis/driver.dart
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,10 @@ class AnalysisDriver implements AnalysisDriverGeneric {
/// The instance of macro executor that is used for all macros.
final macro.MultiMacroExecutor? macroExecutor;

/// The container, shared with other drivers within the same collection,
/// into which all drivers record files ownership.
final OwnedFiles? ownedFiles;

/// The declared environment variables.
final DeclaredVariables declaredVariables;

Expand Down Expand Up @@ -269,6 +273,7 @@ class AnalysisDriver implements AnalysisDriverGeneric {
required Packages packages,
this.macroKernelBuilder,
this.macroExecutor,
this.ownedFiles,
this.analysisContext,
FileContentCache? fileContentCache,
UnlinkedUnitStore? unlinkedUnitStore,
Expand Down Expand Up @@ -1544,6 +1549,7 @@ class AnalysisDriver implements AnalysisDriverGeneric {
unlinkedUnitStore: _unlinkedUnitStore,
prefetchFiles: null,
isGenerated: (_) => false,
onNewFile: _onNewFile,
testData: testView?.fileSystem,
);
_fileTracker = FileTracker(_logger, _fsState, _fileContentStrategy);
Expand Down Expand Up @@ -1777,6 +1783,17 @@ class AnalysisDriver implements AnalysisDriverGeneric {
'missing', errorsResult, AnalysisDriverUnitIndexBuilder());
}

void _onNewFile(FileState file) {
final ownedFiles = this.ownedFiles;
if (ownedFiles != null) {
if (addedFiles.contains(file.path)) {
ownedFiles.addAdded(file.uri, this);
} else {
ownedFiles.addKnown(file.uri, this);
}
}
}

void _removePotentiallyAffectedLibraries(
Set<String> accumulatedAffected,
String path,
Expand Down Expand Up @@ -2405,6 +2422,29 @@ class ExceptionResult {
});
}

/// Container that keeps track of file owners.
class OwnedFiles {
/// Key: the absolute file URI.
/// Value: the driver to which the file is added.
final Map<Uri, AnalysisDriver> addedFiles = {};

/// Key: the absolute file URI.
/// Value: a driver in which this file is available via dependencies.
/// This map does not contain any files that are in [addedFiles].
final Map<Uri, AnalysisDriver> knownFiles = {};

void addAdded(Uri uri, AnalysisDriver analysisDriver) {
addedFiles[uri] ??= analysisDriver;
knownFiles.remove(uri);
}

void addKnown(Uri uri, AnalysisDriver analysisDriver) {
if (!addedFiles.containsKey(uri)) {
knownFiles[uri] = analysisDriver;
}
}
}

/// Worker in [AnalysisDriverScheduler].
abstract class SchedulerWorker {
/// Return the priority of work that this worker needs to perform.
Expand Down
5 changes: 5 additions & 0 deletions pkg/analyzer/lib/src/dart/analysis/file_state.dart
Original file line number Diff line number Diff line change
Expand Up @@ -1150,6 +1150,9 @@ class FileSystemState {
/// of a file that is generated.
final bool Function(String path) isGenerated;

/// The function that is invoked when a new file is created.
final void Function(FileState file) onNewFile;

late final FileSystemStateTestView _testView;

final FileSystemTestData? testData;
Expand All @@ -1169,6 +1172,7 @@ class FileSystemState {
required this.unlinkedUnitStore,
required this.prefetchFiles,
required this.isGenerated,
required this.onNewFile,
required this.testData,
}) {
_testView = FileSystemStateTestView(this);
Expand Down Expand Up @@ -1463,6 +1467,7 @@ class FileSystemState {
knownFiles.add(file);
fileStamp++;
file.refresh();
onNewFile(file);
return file;
}
}
Expand Down
31 changes: 13 additions & 18 deletions pkg/analyzer/lib/src/dart/analysis/search.dart
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,7 @@ class FindDeclarations {
final FuzzyMatcher matcher;
final String? onlyForFile;
final bool onlyAnalyzed;
final OwnedFiles ownedFiles;
final OperationPerformanceImpl performance;

FindDeclarations(
Expand All @@ -157,33 +158,27 @@ class FindDeclarations {
this.maxResults, {
this.onlyForFile,
this.onlyAnalyzed = false,
required this.ownedFiles,
required this.performance,
}) : matcher = FuzzyMatcher(pattern);

Future<void> compute([CancellationToken? cancellationToken]) async {
var searchedFiles = SearchedFiles();
// Add analyzed files first, so priority is given to drivers that analyze
// files over those that just reference them.
for (var driver in drivers) {
searchedFiles.ownAnalyzed(driver.search);
}
if (!onlyAnalyzed) {
await performance.runAsync('discoverAvailableFiles', (performance) async {
await Future.wait(
drivers.map((driver) => driver.discoverAvailableFiles()),
);
});

performance.run('ownKnown', (performance) {
for (var driver in drivers) {
searchedFiles.ownKnown(driver.search);
}
});
}

final entries = [
...ownedFiles.addedFiles.entries,
if (!onlyAnalyzed) ...ownedFiles.knownFiles.entries,
];

await performance.runAsync('findDeclarations', (performance) async {
await _FindDeclarations(
searchedFiles,
entries,
result,
pattern,
matcher,
Expand Down Expand Up @@ -1177,7 +1172,7 @@ class _FindCompilationUnitDeclarations {

/// Searches through [files] for declarations.
class _FindDeclarations {
final SearchedFiles files;
final List<MapEntry<Uri, AnalysisDriver>> fileEntries;
final WorkspaceSymbols result;
final int? maxResults;
final String pattern;
Expand All @@ -1186,7 +1181,7 @@ class _FindDeclarations {
final OperationPerformanceImpl performance;

_FindDeclarations(
this.files,
this.fileEntries,
this.result,
this.pattern,
this.matcher,
Expand All @@ -1209,14 +1204,14 @@ class _FindDeclarations {

var filesProcessed = 0;
try {
for (var entry in files.uriOwners.entries) {
for (var entry in fileEntries) {
var uri = entry.key;
var search = entry.value;
var analysisDriver = entry.value;

final libraryElement = await performance.runAsync(
'getLibraryByUri',
(performance) async {
final result = await search._driver.getLibraryByUri('$uri');
final result = await analysisDriver.getLibraryByUri('$uri');
if (result is LibraryElementResultImpl) {
return result.element as LibraryElementImpl;
}
Expand Down
1 change: 1 addition & 0 deletions pkg/analyzer/lib/src/dart/micro/resolve_file.dart
Original file line number Diff line number Diff line change
Expand Up @@ -742,6 +742,7 @@ class FileResolver {
),
prefetchFiles: prefetchFiles,
isGenerated: isGenerated,
onNewFile: (file) {},
testData: testData?.fileSystem,
unlinkedUnitStore: UnlinkedUnitStoreImpl(),
);
Expand Down
1 change: 1 addition & 0 deletions pkg/analyzer/test/src/dart/analysis/file_state_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -5749,6 +5749,7 @@ class FileSystemStateTest with ResourceProviderMixin {
),
prefetchFiles: null,
isGenerated: (_) => false,
onNewFile: (file) {},
testData: null,
unlinkedUnitStore: UnlinkedUnitStoreImpl(),
);
Expand Down
Loading

0 comments on commit 7d13871

Please sign in to comment.