Cache error results locally in FileResolver.
There is no good mechanism to release these bytes from the shared
cache, we would have to either keep them forever, or release immediately
after we put them (so useless). So, as a compromise, we cache them
space-restrictedly locally.
Change-Id: Iae6227bda31b40374390a6a6bb02501222be4e92
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/246981
Commit-Queue: Konstantin Shcheglov <scheglov@google.com>
Reviewed-by: Keerti Parthasarathy <keertip@google.com>
diff --git a/pkg/analyzer/lib/src/dart/micro/resolve_file.dart b/pkg/analyzer/lib/src/dart/micro/resolve_file.dart
index 3b94d9b..4c83abd 100644
--- a/pkg/analyzer/lib/src/dart/micro/resolve_file.dart
+++ b/pkg/analyzer/lib/src/dart/micro/resolve_file.dart
@@ -12,6 +12,7 @@
import 'package:analyzer/source/line_info.dart';
import 'package:analyzer/src/analysis_options/analysis_options_provider.dart';
import 'package:analyzer/src/context/packages.dart';
+import 'package:analyzer/src/dart/analysis/cache.dart';
import 'package:analyzer/src/dart/analysis/context_root.dart';
import 'package:analyzer/src/dart/analysis/driver.dart' show ErrorEncoding;
import 'package:analyzer/src/dart/analysis/experiments.dart';
@@ -126,6 +127,10 @@
@visibleForTesting
final Map<String, ResolvedLibraryResult> cachedResults = {};
+ /// The cache of error results.
+ final Cache<String, Uint8List> _errorResultsCache =
+ Cache(128 * 1024, (bytes) => bytes.length);
+
FileResolver({
required this.logger,
required this.resourceProvider,
@@ -250,30 +255,28 @@
var errorsSignatureBuilder = ApiSignature();
errorsSignatureBuilder.addBytes(file.libraryCycle.signature);
errorsSignatureBuilder.addBytes(file.digest);
- var errorsSignature = errorsSignatureBuilder.toByteList();
+ final errorsKey = '${errorsSignatureBuilder.toHex()}.errors';
- var errorsKey = '${file.path}.errors';
- var bytes = byteStore.get(errorsKey, errorsSignature)?.bytes;
- List<AnalysisError>? errors;
+ final List<AnalysisError> errors;
+ final bytes = _errorResultsCache.get(errorsKey);
if (bytes != null) {
var data = CiderUnitErrors.fromBuffer(bytes);
errors = data.errors.map((error) {
return ErrorEncoding.decode(file.source, error)!;
}).toList();
- }
-
- if (errors == null) {
+ } else {
var unitResult = await resolve2(
path: path,
performance: performance,
);
errors = unitResult.errors;
- bytes = CiderUnitErrorsBuilder(
- signature: errorsSignature,
- errors: errors.map(ErrorEncoding.encode).toList(),
- ).toBuffer();
- bytes = byteStore.putGet(errorsKey, errorsSignature, bytes).bytes;
+ _errorResultsCache.put(
+ errorsKey,
+ CiderUnitErrorsBuilder(
+ errors: errors.map(ErrorEncoding.encode).toList(),
+ ).toBuffer(),
+ );
}
return ErrorsResultImpl(
diff --git a/pkg/analyzer/lib/src/summary/format.dart b/pkg/analyzer/lib/src/summary/format.dart
index 34f8996..8718728 100644
--- a/pkg/analyzer/lib/src/summary/format.dart
+++ b/pkg/analyzer/lib/src/summary/format.dart
@@ -3217,7 +3217,6 @@
with _CiderUnitErrorsMixin
implements idl.CiderUnitErrors {
List<AnalysisDriverUnitErrorBuilder>? _errors;
- List<int>? _signature;
@override
List<AnalysisDriverUnitErrorBuilder> get errors =>
@@ -3227,19 +3226,8 @@
this._errors = value;
}
- @override
- List<int> get signature => _signature ??= <int>[];
-
- /// The hash signature of this data.
- set signature(List<int> value) {
- assert(value.every((e) => e >= 0));
- this._signature = value;
- }
-
- CiderUnitErrorsBuilder(
- {List<AnalysisDriverUnitErrorBuilder>? errors, List<int>? signature})
- : _errors = errors,
- _signature = signature;
+ CiderUnitErrorsBuilder({List<AnalysisDriverUnitErrorBuilder>? errors})
+ : _errors = errors;
/// Flush [informative] data recursively.
void flushInformative() {
@@ -3248,15 +3236,6 @@
/// Accumulate non-[informative] data into [signature].
void collectApiSignature(api_sig.ApiSignature signatureSink) {
- var signature = this._signature;
- if (signature == null) {
- signatureSink.addInt(0);
- } else {
- signatureSink.addInt(signature.length);
- for (var x in signature) {
- signatureSink.addInt(x);
- }
- }
var errors = this._errors;
if (errors == null) {
signatureSink.addInt(0);
@@ -3275,22 +3254,14 @@
fb.Offset finish(fb.Builder fbBuilder) {
fb.Offset? offset_errors;
- fb.Offset? offset_signature;
var errors = _errors;
if (!(errors == null || errors.isEmpty)) {
offset_errors =
fbBuilder.writeList(errors.map((b) => b.finish(fbBuilder)).toList());
}
- var signature = _signature;
- if (!(signature == null || signature.isEmpty)) {
- offset_signature = fbBuilder.writeListUint32(signature);
- }
fbBuilder.startTable();
if (offset_errors != null) {
- fbBuilder.addOffset(1, offset_errors);
- }
- if (offset_signature != null) {
- fbBuilder.addOffset(0, offset_signature);
+ fbBuilder.addOffset(0, offset_errors);
}
return fbBuilder.endTable();
}
@@ -3318,19 +3289,12 @@
_CiderUnitErrorsImpl(this._bc, this._bcOffset);
List<idl.AnalysisDriverUnitError>? _errors;
- List<int>? _signature;
@override
List<idl.AnalysisDriverUnitError> get errors {
return _errors ??= const fb.ListReader<idl.AnalysisDriverUnitError>(
_AnalysisDriverUnitErrorReader())
- .vTableGet(_bc, _bcOffset, 1, const <idl.AnalysisDriverUnitError>[]);
- }
-
- @override
- List<int> get signature {
- return _signature ??=
- const fb.Uint32ListReader().vTableGet(_bc, _bcOffset, 0, const <int>[]);
+ .vTableGet(_bc, _bcOffset, 0, const <idl.AnalysisDriverUnitError>[]);
}
}
@@ -3342,17 +3306,12 @@
if (local_errors.isNotEmpty) {
result["errors"] = local_errors.map((value) => value.toJson()).toList();
}
- var local_signature = signature;
- if (local_signature.isNotEmpty) {
- result["signature"] = local_signature;
- }
return result;
}
@override
Map<String, Object?> toMap() => {
"errors": errors,
- "signature": signature,
};
@override
diff --git a/pkg/analyzer/lib/src/summary/format.fbs b/pkg/analyzer/lib/src/summary/format.fbs
index 5d58b75..8db2631 100644
--- a/pkg/analyzer/lib/src/summary/format.fbs
+++ b/pkg/analyzer/lib/src/summary/format.fbs
@@ -403,10 +403,7 @@
/// Errors for a single unit.
table CiderUnitErrors {
- errors:[AnalysisDriverUnitError] (id: 1);
-
- /// The hash signature of this data.
- signature:[uint] (id: 0);
+ errors:[AnalysisDriverUnitError] (id: 0);
}
table DiagnosticMessage {
diff --git a/pkg/analyzer/lib/src/summary/idl.dart b/pkg/analyzer/lib/src/summary/idl.dart
index b5f6b8f..13be95e 100644
--- a/pkg/analyzer/lib/src/summary/idl.dart
+++ b/pkg/analyzer/lib/src/summary/idl.dart
@@ -427,12 +427,8 @@
factory CiderUnitErrors.fromBuffer(List<int> buffer) =>
generated.readCiderUnitErrors(buffer);
- @Id(1)
- List<AnalysisDriverUnitError> get errors;
-
- /// The hash signature of this data.
@Id(0)
- List<int> get signature;
+ List<AnalysisDriverUnitError> get errors;
}
abstract class DiagnosticMessage extends base.SummaryClass {
diff --git a/pkg/analyzer/test/src/dart/micro/file_resolution.dart b/pkg/analyzer/test/src/dart/micro/file_resolution.dart
index d9c67b6..f664ce3 100644
--- a/pkg/analyzer/test/src/dart/micro/file_resolution.dart
+++ b/pkg/analyzer/test/src/dart/micro/file_resolution.dart
@@ -35,6 +35,11 @@
Folder get sdkRoot => newFolder('/sdk');
+ File get testFile => getFile(testFilePath);
+
+ @override
+ String get testFilePath => _testFile;
+
@override
void addTestFile(String content) {
newFile(_testFile, content);
diff --git a/pkg/analyzer/test/src/dart/micro/simple_file_resolver_test.dart b/pkg/analyzer/test/src/dart/micro/simple_file_resolver_test.dart
index ffc3b74..4f726b3 100644
--- a/pkg/analyzer/test/src/dart/micro/simple_file_resolver_test.dart
+++ b/pkg/analyzer/test/src/dart/micro/simple_file_resolver_test.dart
@@ -3,6 +3,7 @@
// BSD-style license that can be found in the LICENSE file.
import 'package:analyzer/dart/element/element.dart';
+import 'package:analyzer/file_system/file_system.dart';
import 'package:analyzer/source/line_info.dart';
import 'package:analyzer/src/dart/ast/utilities.dart';
import 'package:analyzer/src/dart/error/syntactic_errors.dart';
@@ -691,79 +692,56 @@
}
test_getErrors_reuse() async {
- addTestFile('var a = b;');
-
- var path = convertPath('/workspace/dart/test/lib/test.dart');
+ newFile(testFilePath, 'var a = b;');
// No resolved files yet.
- expect(fileResolver.testView!.resolvedLibraries, isEmpty);
+ _assertResolvedFiles([]);
// No cached, will resolve once.
expect((await getTestErrors()).errors, hasLength(1));
- expect(fileResolver.testView!.resolvedLibraries, [path]);
+ _assertResolvedFiles([testFile]);
// Has cached, will be not resolved again.
expect((await getTestErrors()).errors, hasLength(1));
- expect(fileResolver.testView!.resolvedLibraries, [path]);
+ _assertResolvedFiles([]);
- // New resolver.
- // Still has cached, will be not resolved.
- createFileResolver();
+ // Change the file, will be resolved again.
+ newFile(testFilePath, 'var a = c;');
+ fileResolver.changeFile(testFile.path);
expect((await getTestErrors()).errors, hasLength(1));
- expect(fileResolver.testView!.resolvedLibraries, <Object>[]);
-
- // Change the file, new resolver.
- // With changed file the previously cached result cannot be used.
- addTestFile('var a = c;');
- createFileResolver();
- expect((await getTestErrors()).errors, hasLength(1));
- expect(fileResolver.testView!.resolvedLibraries, [path]);
-
- // New resolver.
- // Still has cached, will be not resolved.
- createFileResolver();
- expect((await getTestErrors()).errors, hasLength(1));
- expect(fileResolver.testView!.resolvedLibraries, <Object>[]);
+ _assertResolvedFiles([testFile]);
}
test_getErrors_reuse_changeDependency() async {
- newFile('/workspace/dart/test/lib/a.dart', r'''
+ final a = newFile('/workspace/dart/test/lib/a.dart', r'''
var a = 0;
''');
- addTestFile(r'''
+ newFile(testFilePath, r'''
import 'a.dart';
var b = a.foo;
''');
- var path = convertPath('/workspace/dart/test/lib/test.dart');
-
// No resolved files yet.
- expect(fileResolver.testView!.resolvedLibraries, isEmpty);
+ _assertResolvedFiles([]);
// No cached, will resolve once.
expect((await getTestErrors()).errors, hasLength(1));
- expect(fileResolver.testView!.resolvedLibraries, [path]);
+ _assertResolvedFiles([testFile]);
// Has cached, will be not resolved again.
expect((await getTestErrors()).errors, hasLength(1));
- expect(fileResolver.testView!.resolvedLibraries, [path]);
+ _assertResolvedFiles([]);
- // Change the dependency, new resolver.
+ // Change the dependency.
// The signature of the result is different.
// The previously cached result cannot be used.
- newFile('/workspace/dart/test/lib/a.dart', r'''
+ newFile(a.path, r'''
var a = 4.2;
''');
- createFileResolver();
+ fileResolver.changeFile(a.path);
expect((await getTestErrors()).errors, hasLength(1));
- expect(fileResolver.testView!.resolvedLibraries, [path]);
-
- // New resolver.
- // Still has cached, will be not resolved.
- createFileResolver();
- expect((await getTestErrors()).errors, hasLength(1));
- expect(fileResolver.testView!.resolvedLibraries, <Object>[]);
+ _assertResolvedFiles([testFile]);
}
test_getFilesWithTopLevelDeclarations_cached() async {
@@ -1097,25 +1075,23 @@
}
test_resolveFile_cache() async {
- var path = convertPath('/workspace/dart/test/lib/test.dart');
- newFile(path, 'var a = 0;');
+ newFile(testFilePath, 'var a = 0;');
// No resolved files yet.
- var testView = fileResolver.testView!;
- expect(testView.resolvedLibraries, isEmpty);
+ _assertResolvedFiles([]);
- await resolveFile2(path);
+ await resolveFile2(testFile.path);
var result1 = result;
// The file was resolved.
- expect(testView.resolvedLibraries, [path]);
+ _assertResolvedFiles([testFile]);
// The result is cached.
- expect(fileResolver.cachedResults, contains(path));
+ expect(fileResolver.cachedResults, contains(testFile.path));
// Ask again, no changes, not resolved.
- await resolveFile2(path);
- expect(testView.resolvedLibraries, [path]);
+ await resolveFile2(testFile.path);
+ _assertResolvedFiles([]);
// The same result was returned.
expect(result, same(result1));
@@ -1125,36 +1101,33 @@
fileResolver.changeFile(a_path);
// The was a change to a file, no matter which, resolve again.
- await resolveFile2(path);
- expect(testView.resolvedLibraries, [path, path]);
+ await resolveFile2(testFile.path);
+ _assertResolvedFiles([testFile]);
// Get should get a new result.
expect(result, isNot(same(result1)));
}
test_resolveFile_dontCache_whenForCompletion() async {
- var a_path = convertPath('/workspace/dart/test/lib/a.dart');
- newFile(a_path, r'''
+ final a = newFile('/workspace/dart/test/lib/a.dart', r'''
part 'b.dart';
''');
- var b_path = convertPath('/workspace/dart/test/lib/b.dart');
- newFile(b_path, r'''
+ final b = newFile('/workspace/dart/test/lib/b.dart', r'''
part of 'a.dart';
''');
// No resolved files yet.
- var testView = fileResolver.testView!;
- expect(testView.resolvedLibraries, isEmpty);
+ _assertResolvedFiles([]);
await fileResolver.resolve2(
- path: b_path,
+ path: b.path,
completionLine: 0,
completionColumn: 0,
);
- // The file was resolved.
- expect(testView.resolvedLibraries, [a_path]);
+ // The library was resolved.
+ _assertResolvedFiles([a]);
// The completion location was set, so not units are resolved.
// So, the result should not be cached.
@@ -1281,6 +1254,17 @@
expect(fileResolver.fsState!.testView.removedPaths, matcher);
}
+ void _assertResolvedFiles(
+ List<File> expected, {
+ bool andClear = true,
+ }) {
+ final actual = fileResolver.testView!.resolvedLibraries;
+ expect(actual, expected.map((e) => e.path).toList());
+ if (andClear) {
+ actual.clear();
+ }
+ }
+
Future<Element> _findElement(int offset, String filePath) async {
var resolvedUnit = await fileResolver.resolve2(path: filePath);
var node = NodeLocator(offset).searchWithin(resolvedUnit.unit);
