From 8f1a15daca56de4ae6397e194ff254944ed8f482 Mon Sep 17 00:00:00 2001 From: Ilya Yanok Date: Fri, 3 Jun 2022 07:14:06 +0000 Subject: [PATCH] Revert "Replace CiderByteStore methods with variants without signature." This reverts commit 3a1a08106ef9f99d2bb6a5886e0c40bf9651767d. Reason for revert: breaks dartd, can you maybe add instead of replace? Original change's description: > Replace CiderByteStore methods with variants without signature. > > Change-Id: I42c3aa456554974c1f44b95833e14487542539a5 > Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/246986 > Commit-Queue: Konstantin Shcheglov > Reviewed-by: Keerti Parthasarathy TBR=keertip@google.com,scheglov@google.com Change-Id: Ia05a618cde2f87b9c3162e28709de31baed18796 No-Presubmit: true No-Tree-Checks: true No-Try: true Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/247081 Reviewed-by: Ilya Yanok Reviewed-by: Alexander Thomas Commit-Queue: Ilya Yanok --- .../lib/src/dart/micro/cider_byte_store.dart | 56 ++++++++++-------- .../lib/src/dart/micro/library_graph.dart | 34 ++++++----- .../lib/src/dart/micro/resolve_file.dart | 59 ++++++++++--------- .../dart/micro/simple_file_resolver_test.dart | 2 +- 4 files changed, 82 insertions(+), 69 deletions(-) diff --git a/pkg/analyzer/lib/src/dart/micro/cider_byte_store.dart b/pkg/analyzer/lib/src/dart/micro/cider_byte_store.dart index d52d6aa2650..0ffa205bf1c 100644 --- a/pkg/analyzer/lib/src/dart/micro/cider_byte_store.dart +++ b/pkg/analyzer/lib/src/dart/micro/cider_byte_store.dart @@ -5,6 +5,7 @@ import 'dart:typed_data'; import 'package:analyzer/src/dart/analysis/cache.dart'; +import 'package:collection/collection.dart'; class CacheData { final int id; @@ -22,25 +23,18 @@ class CacheData { /// Note that associations are not guaranteed to be persistent. The value /// associated with a key can change or become `null` at any point in time. abstract class CiderByteStore { - /// Return the bytes associated with the [key], and increment the reference - /// count. + /// Return the bytes associated with the errors for given [key] and + /// [signature]. /// /// Return `null` if the association does not exist. - Uint8List? get2(String key); + CacheData? get(String key, Uint8List signature); - /// Associate [bytes] with [key]. - /// Return an internalized version of [bytes], the reference count is `1`. - /// - /// This method will throw an exception if there is already an association - /// for the [key]. The client should either use [get2] to access data, - /// or first [release2] it. - Uint8List putGet2(String key, Uint8List bytes); + /// Associate the given [bytes] with the [key] and [signature]. Return the + /// [CacheData]. + CacheData putGet(String key, Uint8List signature, Uint8List bytes); /// Used to decrement reference count for the given ids, if implemented. void release(Iterable ids); - - /// Decrement the reference count for every key in [keys]. - void release2(Iterable keys); } class CiderByteStoreTestView { @@ -48,33 +42,45 @@ class CiderByteStoreTestView { } class CiderCachedByteStore implements CiderByteStore { - final Cache _cache; + final Cache _cache; + int idCounter = 0; /// This field gets value only during testing. CiderByteStoreTestView? testView; CiderCachedByteStore(int maxCacheSize) - : _cache = Cache(maxCacheSize, (v) => v.length); + : _cache = Cache( + maxCacheSize, (v) => v.data.bytes.length); @override - Uint8List? get2(String key) { - return _cache.get(key); + CacheData? get(String key, Uint8List signature) { + final entry = _cache.get(key); + + if (entry != null && + const ListEquality().equals(entry.signature, signature)) { + return entry.data; + } + return null; } @override - Uint8List putGet2(String key, Uint8List bytes) { - _cache.put(key, bytes); + CacheData putGet(String key, Uint8List signature, Uint8List bytes) { + idCounter++; + var entry = CiderCacheEntry(signature, CacheData(idCounter, bytes)); + _cache.put(key, entry); testView?.length++; - return bytes; + return entry.data; } @override void release(Iterable ids) { // do nothing } - - @override - void release2(Iterable keys) { - // TODO(scheglov) implement - } +} + +class CiderCacheEntry { + final CacheData data; + final Uint8List signature; + + CiderCacheEntry(this.signature, this.data); } diff --git a/pkg/analyzer/lib/src/dart/micro/library_graph.dart b/pkg/analyzer/lib/src/dart/micro/library_graph.dart index f7e8c7be9f3..4a51ff166c2 100644 --- a/pkg/analyzer/lib/src/dart/micro/library_graph.dart +++ b/pkg/analyzer/lib/src/dart/micro/library_graph.dart @@ -139,7 +139,7 @@ class FileState { Source get source => _location.source; - String get unlinkedKey => _unlinked.unlinkedKey; + int get unlinkedId => _unlinked.unlinkedId; UnlinkedUnit get unlinkedUnit => _unlinked.unlinked.unit; @@ -330,12 +330,12 @@ class FileSystemState { } } - /// Clears all the cached files. Returns the list of keys of all the removed + /// Clears all the cached files. Returns the list of ids of all the removed /// files. - Set collectSharedDataKeys() { - var result = {}; + Set collectSharedDataIdentifiers() { + var result = {}; for (var file in _pathToFile.values) { - result.add(file._unlinked.unlinkedKey); + result.add(file._unlinked.unlinkedId); } return result; } @@ -560,9 +560,9 @@ class LibraryCycle { /// The hash of all the paths of the files in this cycle. late String cyclePathsHash; - /// The key of the resolution cache entry. + /// The ID of the resolution cache entry. /// It is `null` if we failed to load libraries of the cycle. - String? resolutionKey; + int? resolutionId; LibraryCycle(); @@ -713,8 +713,8 @@ class _FileStateUnlinked { final bool exists; final CiderUnlinkedUnit unlinked; - /// Key of the cache entry with unlinked data. - final String unlinkedKey; + /// id of the cache entry with unlinked data. + final int unlinkedId; factory _FileStateUnlinked({ required _FileStateLocation location, @@ -723,6 +723,7 @@ class _FileStateUnlinked { }) { location._fsState.testView.refreshedFiles.add(location.path); + int unlinkedId; CiderUnlinkedUnit unlinked; var digest = performance.run('digest', (performance) { @@ -733,14 +734,15 @@ class _FileStateUnlinked { var exists = digest.isNotEmpty; - final unlinkedKey = '${hex.encode(digest)}.unlinked'; + var unlinkedKey = '${location.path}.unlinked'; var isUnlinkedFromCache = true; // Prepare bytes of the unlinked bundle - existing or new. // TODO(migration): should not be nullable Uint8List? unlinkedBytes; { - unlinkedBytes = location._fsState._byteStore.get2(unlinkedKey); + var unlinkedData = location._fsState._byteStore.get(unlinkedKey, digest); + unlinkedBytes = unlinkedData?.bytes; if (unlinkedBytes == null || unlinkedBytes.isEmpty) { isUnlinkedFromCache = false; @@ -761,12 +763,14 @@ class _FileStateUnlinked { var unlinkedUnit = serializeAstCiderUnlinked(unit); unlinkedBytes = unlinkedUnit.toBytes(); performance.getDataInt('length').add(unlinkedBytes!.length); - unlinkedBytes = - location._fsState._byteStore.putGet2(unlinkedKey, unlinkedBytes!); + unlinkedData = location._fsState._byteStore + .putGet(unlinkedKey, digest, unlinkedBytes!); + unlinkedBytes = unlinkedData!.bytes; }); unlinked = CiderUnlinkedUnit.fromBytes(unlinkedBytes!); } + unlinkedId = unlinkedData!.id; } // Read the unlinked bundle. @@ -778,7 +782,7 @@ class _FileStateUnlinked { digest: digest, exists: exists, unlinked: unlinked, - unlinkedKey: unlinkedKey, + unlinkedId: unlinkedId, ); if (isUnlinkedFromCache) { performance.run('prefetch', (_) { @@ -794,7 +798,7 @@ class _FileStateUnlinked { required this.digest, required this.exists, required this.unlinked, - required this.unlinkedKey, + required this.unlinkedId, }) : _partOfLibrary = partOfLibrary; FileState? get partOfLibrary { diff --git a/pkg/analyzer/lib/src/dart/micro/resolve_file.dart b/pkg/analyzer/lib/src/dart/micro/resolve_file.dart index 670449148a2..4c83abdae77 100644 --- a/pkg/analyzer/lib/src/dart/micro/resolve_file.dart +++ b/pkg/analyzer/lib/src/dart/micro/resolve_file.dart @@ -114,10 +114,10 @@ class FileResolver { _LibraryContext? libraryContext; - /// List of keys for cache elements that are invalidated. Track elements that + /// List of ids for cache elements that are invalidated. Track elements that /// are invalidated during [changeFile]. Used in [releaseAndClearRemovedIds] /// to release the cache items and is then cleared. - final Set removedCacheKeys = {}; + final Set removedCacheIds = {}; /// The cache of file results, cleared on [changeFile]. /// @@ -173,21 +173,21 @@ class FileResolver { // Schedule disposing references to cached unlinked data. for (var removedFile in removedFiles) { - removedCacheKeys.add(removedFile.unlinkedKey); + removedCacheIds.add(removedFile.unlinkedId); } // Remove libraries represented by removed files. // If we need these libraries later, we will relink and reattach them. if (libraryContext != null) { - libraryContext!.remove(removedFiles, removedCacheKeys); + libraryContext!.remove(removedFiles, removedCacheIds); } } /// Collects all the cached artifacts and add all the cache id's for the - /// removed artifacts to [removedCacheKeys]. + /// removed artifacts to [removedCacheIds]. void collectSharedDataIdentifiers() { - removedCacheKeys.addAll(fsState!.collectSharedDataKeys()); - removedCacheKeys.addAll(libraryContext!.collectSharedDataKeys()); + removedCacheIds.addAll(fsState!.collectSharedDataIdentifiers()); + removedCacheIds.addAll(libraryContext!.collectSharedDataIdentifiers()); } /// Looks for references to the given Element. All the files currently @@ -252,7 +252,7 @@ class FileResolver { ); var file = fileContext.file; - final errorsSignatureBuilder = ApiSignature(); + var errorsSignatureBuilder = ApiSignature(); errorsSignatureBuilder.addBytes(file.libraryCycle.signature); errorsSignatureBuilder.addBytes(file.digest); final errorsKey = '${errorsSignatureBuilder.toHex()}.errors'; @@ -414,10 +414,10 @@ class FileResolver { _resetContextObjects(); } - /// Releases from the cache and clear [removedCacheKeys]. + /// Update the cache with list of invalidated ids and clears [removedCacheIds]. void releaseAndClearRemovedIds() { - byteStore.release2(removedCacheKeys); - removedCacheKeys.clear(); + byteStore.release(removedCacheIds); + removedCacheIds.clear(); } /// Remove cached [FileState]'s that were not used in the current analysis @@ -427,7 +427,7 @@ class FileResolver { void removeFilesNotNecessaryForAnalysisOf(List files) { var removedFiles = fsState!.removeUnusedFiles(files); for (var removedFile in removedFiles) { - removedCacheKeys.add(removedFile.unlinkedKey); + removedCacheIds.add(removedFile.unlinkedId); } } @@ -829,20 +829,20 @@ class _LibraryContext { /// Clears all the loaded libraries. Returns the cache ids for the removed /// artifacts. - Set collectSharedDataKeys() { - var keySet = {}; + Set collectSharedDataIdentifiers() { + var idSet = {}; - void addIfNotNull(String? key) { - if (key != null) { - keySet.add(key); + void addIfNotNull(int? id) { + if (id != null) { + idSet.add(id); } } for (var cycle in loadedBundles) { - addIfNotNull(cycle.resolutionKey); + addIfNotNull(cycle.resolutionId); } loadedBundles.clear(); - return keySet; + return idSet; } /// Load data required to access elements of the given [targetLibrary]. @@ -864,8 +864,9 @@ class _LibraryContext { await loadBundle(directDependency); } - var resolutionKey = '${cycle.signatureStr}.resolution'; - var resolutionBytes = byteStore.get2(resolutionKey); + var resolutionKey = '${cycle.cyclePathsHash}.resolution'; + var resolutionData = byteStore.get(resolutionKey, cycle.signature); + var resolutionBytes = resolutionData?.bytes; var unitsInformativeBytes = {}; for (var library in cycle.libraries) { @@ -938,7 +939,9 @@ class _LibraryContext { librariesLinked += cycle.libraries.length; resolutionBytes = linkResult.resolutionBytes; - resolutionBytes = byteStore.putGet2(resolutionKey, resolutionBytes); + resolutionData = + byteStore.putGet(resolutionKey, cycle.signature, resolutionBytes); + resolutionBytes = resolutionData.bytes; performance.getDataInt('bytesPut').add(resolutionBytes.length); librariesLinkedTimer.stop(); @@ -953,7 +956,7 @@ class _LibraryContext { ), ); } - cycle.resolutionKey = resolutionKey; + cycle.resolutionId = resolutionData!.id; // We might have just linked dart:core, ensure the type provider. _createElementFactoryTypeProvider(); @@ -972,22 +975,22 @@ class _LibraryContext { /// Remove libraries represented by the [removed] files. /// If we need these libraries later, we will relink and reattach them. - void remove(List removed, Set removedKeys) { + void remove(List removed, Set removedIds) { elementFactory.removeLibraries( removed.map((e) => e.uriStr).toSet(), ); var removedSet = removed.toSet(); - void addIfNotNull(String? key) { - if (key != null) { - removedKeys.add(key); + void addIfNotNull(int? id) { + if (id != null) { + removedIds.add(id); } } loadedBundles.removeWhere((cycle) { if (cycle.libraries.any(removedSet.contains)) { - addIfNotNull(cycle.resolutionKey); + addIfNotNull(cycle.resolutionId); return true; } return false; 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 31e77485714..4f726b32162 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 @@ -337,7 +337,7 @@ class A {} await resolveFile(aPath); fileResolver.collectSharedDataIdentifiers(); - expect(fileResolver.removedCacheKeys.length, + expect(fileResolver.removedCacheIds.length, (fileResolver.byteStore as CiderCachedByteStore).testView!.length); }