Use explicit FileContentCache instance for AnalysisContextCollectionImpl.

Previously we cached file contents implicitly, using Expando on top
of ResourceProvider. This is somewhat dangerous, and does not allow
us to decide when to cache, and when to invalidate.

Change-Id: I5c93ac22f217bb2baba6b7f6e73fc05f1c674e75
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/201233
Reviewed-by: Samuel Rawlins <srawlins@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Konstantin Shcheglov <scheglov@google.com>
This commit is contained in:
Konstantin Shcheglov
2021-05-26 00:54:01 +00:00
committed by commit-bot@chromium.org
parent d97bd97957
commit d2682070f4
10 changed files with 161 additions and 119 deletions
@@ -40,6 +40,7 @@ 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/file_byte_store.dart'
show EvictingFileByteStore;
import 'package:analyzer/src/dart/analysis/file_content_cache.dart';
import 'package:analyzer/src/dart/analysis/performance_logger.dart';
import 'package:analyzer/src/dart/ast/element_locator.dart';
import 'package:analyzer/src/dart/ast/utilities.dart';
@@ -79,6 +80,7 @@ abstract class AbstractAnalysisServer {
late final SearchEngine searchEngine;
late ByteStore byteStore;
late FileContentCache fileContentCache;
late analysis.AnalysisDriverScheduler analysisDriverScheduler;
@@ -181,6 +183,7 @@ abstract class AbstractAnalysisServer {
this.analysisPerformanceLogger = PerformanceLog(sink);
byteStore = createByteStore(resourceProvider);
fileContentCache = FileContentCache(resourceProvider);
analysisDriverScheduler = analysis.AnalysisDriverScheduler(
analysisPerformanceLogger,
@@ -198,6 +201,7 @@ abstract class AbstractAnalysisServer {
resourceProvider,
sdkManager,
byteStore,
fileContentCache,
analysisPerformanceLogger,
analysisDriverScheduler,
instrumentationService,
@@ -14,6 +14,7 @@ import 'package:analyzer/src/dart/analysis/analysis_context_collection.dart';
import 'package:analyzer/src/dart/analysis/byte_store.dart';
import 'package:analyzer/src/dart/analysis/driver.dart';
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';
import 'package:analyzer/src/generated/java_engine.dart';
import 'package:analyzer/src/generated/sdk.dart';
@@ -138,6 +139,9 @@ class ContextManagerImpl implements ContextManager {
/// The storage for cached results.
final ByteStore _byteStore;
/// The cache of file contents shared between context of the collection.
final FileContentCache _fileContentCache;
/// The logger used to create analysis contexts.
final PerformanceLog _performanceLog;
@@ -196,8 +200,14 @@ class ContextManagerImpl implements ContextManager {
/// clean up when we destroy a context.
final bazelWatchedPathsPerFolder = <Folder, _BazelWatchedFiles>{};
ContextManagerImpl(this.resourceProvider, this.sdkManager, this._byteStore,
this._performanceLog, this._scheduler, this._instrumentationService,
ContextManagerImpl(
this.resourceProvider,
this.sdkManager,
this._byteStore,
this._fileContentCache,
this._performanceLog,
this._scheduler,
this._instrumentationService,
{required enableBazelWatcher})
: pathContext = resourceProvider.pathContext {
if (enableBazelWatcher) {
@@ -406,6 +416,7 @@ class ContextManagerImpl implements ContextManager {
resourceProvider: resourceProvider,
scheduler: _scheduler,
sdkPath: sdkManager.defaultSdkDirectory,
fileContentCache: _fileContentCache,
);
for (var analysisContext in collection.contexts) {
+8 -2
View File
@@ -13,6 +13,7 @@ import 'package:analyzer/src/context/packages.dart';
import 'package:analyzer/src/dart/analysis/byte_store.dart';
import 'package:analyzer/src/dart/analysis/driver.dart'
show AnalysisDriver, AnalysisDriverScheduler;
import 'package:analyzer/src/dart/analysis/file_content_cache.dart';
import 'package:analyzer/src/dart/analysis/performance_logger.dart';
import 'package:analyzer/src/dart/sdk/sdk.dart';
import 'package:analyzer/src/generated/engine.dart';
@@ -89,8 +90,12 @@ class ContextBuilder {
/// Return an analysis driver that is configured correctly to analyze code in
/// the directory with the given [path].
AnalysisDriver buildDriver(ContextRoot contextRoot, Workspace workspace,
{void Function(AnalysisOptionsImpl)? updateAnalysisOptions}) {
AnalysisDriver buildDriver(
ContextRoot contextRoot,
Workspace workspace, {
void Function(AnalysisOptionsImpl)? updateAnalysisOptions,
FileContentCache? fileContentCache,
}) {
String path = contextRoot.root;
var options = getAnalysisOptions(path, workspace, contextRoot: contextRoot);
@@ -124,6 +129,7 @@ class ContextBuilder {
enableIndex: enableIndex,
externalSummaries: summaryData,
retainDataForTesting: retainDataForTesting,
fileContentCache: fileContentCache,
);
declareVariablesInDriver(driver);
@@ -11,6 +11,7 @@ import 'package:analyzer/src/dart/analysis/byte_store.dart';
import 'package:analyzer/src/dart/analysis/context_builder.dart';
import 'package:analyzer/src/dart/analysis/driver.dart';
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';
import 'package:analyzer/src/generated/engine.dart' show AnalysisOptionsImpl;
import 'package:cli_util/cli_util.dart';
@@ -39,6 +40,7 @@ class AnalysisContextCollectionImpl implements AnalysisContextCollection {
bool retainDataForTesting = false,
String? sdkPath,
AnalysisDriverScheduler? scheduler,
FileContentCache? fileContentCache,
void Function(AnalysisOptionsImpl)? updateAnalysisOptions,
}) : resourceProvider =
resourceProvider ?? PhysicalResourceProvider.INSTANCE {
@@ -71,6 +73,7 @@ class AnalysisContextCollectionImpl implements AnalysisContextCollection {
sdkPath: sdkPath,
scheduler: scheduler,
updateAnalysisOptions: updateAnalysisOptions,
fileContentCache: fileContentCache,
);
contexts.add(context);
}
@@ -15,6 +15,7 @@ import 'package:analyzer/src/dart/analysis/byte_store.dart'
import 'package:analyzer/src/dart/analysis/driver.dart'
show AnalysisDriver, AnalysisDriverScheduler;
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'
show PerformanceLog;
import 'package:analyzer/src/generated/engine.dart' show AnalysisOptionsImpl;
@@ -47,6 +48,7 @@ class ContextBuilderImpl implements ContextBuilder {
String? sdkPath,
String? sdkSummaryPath,
void Function(AnalysisOptionsImpl)? updateAnalysisOptions,
FileContentCache? fileContentCache,
}) {
// TODO(scheglov) Remove this, and make `sdkPath` required.
sdkPath ??= getSdkPath();
@@ -91,6 +93,7 @@ class ContextBuilderImpl implements ContextBuilder {
AnalysisDriver driver = builder.buildDriver(
oldContextRoot,
contextRoot.workspace,
fileContentCache: fileContentCache,
updateAnalysisOptions: updateAnalysisOptions,
);
@@ -19,6 +19,7 @@ import 'package:analyzer/src/context/context_root.dart';
import 'package:analyzer/src/context/packages.dart';
import 'package:analyzer/src/dart/analysis/byte_store.dart';
import 'package:analyzer/src/dart/analysis/feature_set_provider.dart';
import 'package:analyzer/src/dart/analysis/file_content_cache.dart';
import 'package:analyzer/src/dart/analysis/file_state.dart';
import 'package:analyzer/src/dart/analysis/file_tracker.dart';
import 'package:analyzer/src/dart/analysis/index.dart';
@@ -111,7 +112,7 @@ class AnalysisDriver implements AnalysisDriverGeneric {
/// This [ContentCache] is consulted for a file content before reading
/// the content from the file.
final FileContentOverlay? _contentOverlay;
final FileContentCache _fileContentCache;
/// The analysis options to analyze with.
AnalysisOptionsImpl _analysisOptions;
@@ -302,13 +303,15 @@ class AnalysisDriver implements AnalysisDriverGeneric {
required SourceFactory sourceFactory,
required AnalysisOptionsImpl analysisOptions,
required Packages packages,
FileContentCache? fileContentCache,
bool enableIndex = false,
SummaryDataStore? externalSummaries,
bool retainDataForTesting = false,
}) : _scheduler = scheduler,
_resourceProvider = resourceProvider,
_byteStore = byteStore,
_contentOverlay = FileContentOverlay(),
_fileContentCache =
fileContentCache ?? FileContentCache.ephemeral(resourceProvider),
_analysisOptions = analysisOptions,
enableIndex = enableIndex,
_logger = logger,
@@ -1806,7 +1809,6 @@ class AnalysisDriver implements AnalysisDriverGeneric {
_fsState = FileSystemState(
_logger,
_byteStore,
_contentOverlay,
_resourceProvider,
name,
sourceFactory,
@@ -1817,6 +1819,7 @@ class AnalysisDriver implements AnalysisDriverGeneric {
_saltForElements,
featureSetProvider,
externalSummaries: _externalSummaries,
fileContentCache: _fileContentCache,
);
_fileTracker = FileTracker(_logger, _fsState, _changeHook);
}
@@ -0,0 +1,105 @@
// Copyright (c) 2021, 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 'dart:convert';
import 'dart:typed_data';
import 'package:analyzer/file_system/file_system.dart';
import 'package:convert/convert.dart';
import 'package:crypto/crypto.dart';
/// Information about the content of a file.
class FileContent {
final String path;
final bool exists;
final String content;
final String contentHash;
FileContent._(this.path, this.exists, this.content, this.contentHash);
}
/// The cache of information about content of files.
abstract class FileContentCache {
final ResourceProvider _resourceProvider;
factory FileContentCache(ResourceProvider resourceProvider) {
return _FileContentCacheImpl(resourceProvider);
}
factory FileContentCache.ephemeral(ResourceProvider resourceProvider) {
return _FileContentCacheEphemeral(resourceProvider);
}
FileContentCache._(this._resourceProvider);
/// Return the content of the file with the given [path].
FileContent get(String path);
/// Discard the cache value for the file with the given [path].
void invalidate(String path) {}
void invalidateAll() {}
FileContent _read(String path) {
List<int> contentBytes;
String content;
bool exists;
try {
contentBytes = _resourceProvider.getFile(path).readAsBytesSync();
content = utf8.decode(contentBytes);
exists = true;
} catch (_) {
contentBytes = Uint8List(0);
content = '';
exists = false;
}
List<int> contentHashBytes = md5.convert(contentBytes).bytes;
String contentHash = hex.encode(contentHashBytes);
return FileContent._(path, exists, content, contentHash);
}
}
/// [FileContentCache] that caches never.
class _FileContentCacheEphemeral extends FileContentCache {
_FileContentCacheEphemeral(ResourceProvider resourceProvider)
: super._(resourceProvider);
@override
FileContent get(String path) {
return _read(path);
}
}
/// [FileContentCache] that caches forever.
class _FileContentCacheImpl extends FileContentCache {
final Map<String, FileContent> _pathToFile = {};
_FileContentCacheImpl(ResourceProvider resourceProvider)
: super._(resourceProvider);
@override
FileContent get(String path) {
var file = _pathToFile[path];
if (file != null) {
return file;
}
file = _read(path);
_pathToFile[path] = file;
return file;
}
@override
void invalidate(String path) {
_pathToFile.remove(path);
}
@override
void invalidateAll() {
_pathToFile.clear();
}
}
@@ -2,7 +2,6 @@
// 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 'dart:convert';
import 'dart:typed_data';
import 'package:_fe_analyzer_shared/src/scanner/token_impl.dart'
@@ -17,6 +16,7 @@ import 'package:analyzer/file_system/file_system.dart';
import 'package:analyzer/src/dart/analysis/byte_store.dart';
import 'package:analyzer/src/dart/analysis/defined_names.dart';
import 'package:analyzer/src/dart/analysis/feature_set_provider.dart';
import 'package:analyzer/src/dart/analysis/file_content_cache.dart';
import 'package:analyzer/src/dart/analysis/library_graph.dart';
import 'package:analyzer/src/dart/analysis/performance_logger.dart';
import 'package:analyzer/src/dart/analysis/referenced_names.dart';
@@ -38,7 +38,6 @@ import 'package:analyzer/src/util/either.dart';
import 'package:analyzer/src/workspace/workspace.dart';
import 'package:collection/collection.dart';
import 'package:convert/convert.dart';
import 'package:crypto/crypto.dart';
import 'package:meta/meta.dart';
import 'package:pub_semver/pub_semver.dart';
@@ -421,8 +420,12 @@ class FileState {
_invalidateCurrentUnresolvedData();
if (!allowCached) {
_fsState.markFileForReading(path);
}
{
var rawFileState = _fsState._fileContentCache.get(path, allowCached);
var rawFileState = _fsState._fileContentCache.get(path);
_content = rawFileState.content;
_exists = rawFileState.exists;
_contentHash = rawFileState.contentHash;
@@ -716,7 +719,6 @@ class FileSystemState {
final ResourceProvider _resourceProvider;
final String contextName;
final ByteStore _byteStore;
final FileContentOverlay? _contentOverlay;
final SourceFactory _sourceFactory;
final Workspace? _workspace;
final DeclaredVariables _declaredVariables;
@@ -767,15 +769,14 @@ class FileSystemState {
int fileStamp = 0;
/// The cache of content of files, possibly shared with other file system
/// states with the same resource provider and the content overlay.
late final _FileContentCache _fileContentCache;
/// states.
final FileContentCache _fileContentCache;
late final FileSystemStateTestView _testView;
FileSystemState(
this._logger,
this._byteStore,
this._contentOverlay,
this._resourceProvider,
this.contextName,
this._sourceFactory,
@@ -787,11 +788,8 @@ class FileSystemState {
this._saltForElements,
this.featureSetProvider, {
this.externalSummaries,
}) {
_fileContentCache = _FileContentCache.getInstance(
_resourceProvider,
_contentOverlay,
);
required FileContentCache fileContentCache,
}) : _fileContentCache = fileContentCache {
_testView = FileSystemStateTestView(this);
}
@@ -946,7 +944,7 @@ class FileSystemState {
/// The file with the given [path] might have changed, so ensure that it is
/// read the next time it is refreshed.
void markFileForReading(String path) {
_fileContentCache.remove(path);
_fileContentCache.invalidate(path);
}
void readPartsForLibraries() {
@@ -973,7 +971,7 @@ class FileSystemState {
/// will be built.
void resetUriResolution() {
_sourceFactory.clearCache();
_fileContentCache.clear();
_fileContentCache.invalidateAll();
_clearFiles();
}
@@ -1013,99 +1011,3 @@ class FileSystemStateTestView {
.toSet();
}
}
/// Information about the content of a file.
class _FileContent {
final String path;
final bool exists;
final String content;
final String contentHash;
_FileContent(this.path, this.exists, this.content, this.contentHash);
}
/// The cache of information about content of files.
class _FileContentCache {
/// Weak map of cache instances.
///
/// Outer key is a [FileContentOverlay].
/// Inner key is a [ResourceProvider].
static final _instances = Expando<Expando<_FileContentCache>>();
/// Weak map of cache instances.
///
/// Key is a [ResourceProvider].
static final _instances2 = Expando<_FileContentCache>();
final ResourceProvider _resourceProvider;
final FileContentOverlay? _contentOverlay;
final Map<String, _FileContent> _pathToFile = {};
_FileContentCache(this._resourceProvider, this._contentOverlay);
void clear() {
_pathToFile.clear();
}
/// Return the content of the file with the given [path].
///
/// If [allowCached] is `true`, and the file is in the cache, return the
/// cached data. Otherwise read the file, compute and cache the data.
_FileContent get(String path, bool allowCached) {
var file = allowCached ? _pathToFile[path] : null;
if (file == null) {
List<int> contentBytes;
String? content;
bool exists;
try {
if (_contentOverlay != null) {
content = _contentOverlay![path];
}
if (content != null) {
contentBytes = utf8.encode(content);
} else {
contentBytes = _resourceProvider.getFile(path).readAsBytesSync();
content = utf8.decode(contentBytes);
}
exists = true;
} catch (_) {
contentBytes = Uint8List(0);
content = '';
exists = false;
}
List<int> contentHashBytes = md5.convert(contentBytes).bytes;
String contentHash = hex.encode(contentHashBytes);
file = _FileContent(path, exists, content, contentHash);
_pathToFile[path] = file;
}
return file;
}
/// Remove the file with the given [path] from the cache.
void remove(String path) {
_pathToFile.remove(path);
}
static _FileContentCache getInstance(
ResourceProvider resourceProvider, FileContentOverlay? contentOverlay) {
Expando<_FileContentCache>? providerToInstance;
if (contentOverlay != null) {
providerToInstance = _instances[contentOverlay];
if (providerToInstance == null) {
providerToInstance = Expando<_FileContentCache>();
_instances[contentOverlay] = providerToInstance;
}
} else {
providerToInstance = _instances2;
}
var instance = providerToInstance[resourceProvider];
if (instance == null) {
instance = _FileContentCache(resourceProvider, contentOverlay);
providerToInstance[resourceProvider] = instance;
}
return instance;
}
}
@@ -12,6 +12,7 @@ import 'package:analyzer/src/context/packages.dart';
import 'package:analyzer/src/dart/analysis/byte_store.dart';
import 'package:analyzer/src/dart/analysis/experiments.dart';
import 'package:analyzer/src/dart/analysis/feature_set_provider.dart';
import 'package:analyzer/src/dart/analysis/file_content_cache.dart';
import 'package:analyzer/src/dart/analysis/file_state.dart';
import 'package:analyzer/src/dart/analysis/library_graph.dart';
import 'package:analyzer/src/dart/analysis/performance_logger.dart';
@@ -83,7 +84,6 @@ class FileSystemStateTest with ResourceProviderMixin {
fileSystemState = FileSystemState(
logger,
byteStore,
contentOverlay,
resourceProvider,
'contextName',
sourceFactory,
@@ -93,6 +93,7 @@ class FileSystemStateTest with ResourceProviderMixin {
Uint32List(0),
Uint32List(0),
featureSetProvider,
fileContentCache: FileContentCache.ephemeral(resourceProvider),
);
}
+5 -1
View File
@@ -13,6 +13,7 @@ import 'package:analyzer/src/dart/analysis/analysis_context_collection.dart';
import 'package:analyzer/src/dart/analysis/byte_store.dart';
import 'package:analyzer/src/dart/analysis/driver.dart';
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/file_state.dart';
import 'package:analyzer/src/dart/analysis/results.dart';
import 'package:analyzer/src/generated/engine.dart';
@@ -485,6 +486,7 @@ class Driver implements CommandLineStarter {
class _AnalysisContextProvider {
final ResourceProvider _resourceProvider;
final FileContentCache _fileContentCache;
CommandLineOptions _commandLineOptions;
List<String> _pathList;
@@ -493,7 +495,8 @@ class _AnalysisContextProvider {
AnalysisContextCollectionImpl _collection;
DriverBasedAnalysisContext _analysisContext;
_AnalysisContextProvider(this._resourceProvider);
_AnalysisContextProvider(this._resourceProvider)
: _fileContentCache = FileContentCache(_resourceProvider);
DriverBasedAnalysisContext get analysisContext {
return _analysisContext;
@@ -550,6 +553,7 @@ class _AnalysisContextProvider {
resourceProvider: _resourceProvider,
sdkPath: _commandLineOptions.dartSdkPath,
updateAnalysisOptions: _updateAnalysisOptions,
fileContentCache: _fileContentCache,
);
_setContextForPath(path);