Make ByteStore similar to CiderByteStore and switch Cider to it.

Change-Id: Ic05df27da094fdd5671eb45d66ab3533b0f0d844
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/248445
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Konstantin Shcheglov <scheglov@google.com>

pkg/analysis_server/test/src/cider/cider_service.dart

@@ -4,8 +4,8 @@
 
 import 'dart:convert';
 
+import 'package:analyzer/src/dart/analysis/byte_store.dart';
 import 'package:analyzer/src/dart/analysis/performance_logger.dart';
-import 'package:analyzer/src/dart/micro/cider_byte_store.dart';
 import 'package:analyzer/src/dart/micro/resolve_file.dart';
 import 'package:analyzer/src/dart/sdk/sdk.dart';
 import 'package:analyzer/src/test_utilities/mock_sdk.dart';
@@ -43,7 +43,7 @@ class CiderServiceTest with ResourceProviderMixin {
       getFileDigest: (String path) => _getDigest(path),
       prefetchFiles: null,
       workspace: workspace,
-      byteStore: MemoryCiderByteStore(),
+      byteStore: MemoryByteStore(),
     );
     fileResolver.testView = FileResolverTestView();
   }

pkg/analyzer/lib/src/dart/analysis/byte_store.dart

@@ -5,6 +5,7 @@
 import 'dart:typed_data';
 
 import 'package:analyzer/src/dart/analysis/cache.dart';
+import 'package:meta/meta.dart';
 
 /// Store of bytes associated with string keys.
 ///
@@ -14,29 +15,72 @@ import 'package:analyzer/src/dart/analysis/cache.dart';
 ///
 /// 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.
-///
-/// TODO(scheglov) Research using asynchronous API.
 abstract class ByteStore {
   /// Return the bytes associated with the given [key].
   /// Return `null` if the association does not exist.
+  ///
+  /// If this store supports reference counting, increments it.
   Uint8List? get(String key);
 
-  /// Associate the given [bytes] with the [key].
-  void put(String key, Uint8List bytes);
+  /// Associate [bytes] with [key].
+  ///
+  /// If this store supports reference counting, returns the internalized
+  /// version of [bytes], the reference count is set to `1`.
+  ///
+  /// TODO(scheglov) Disable overwriting.
+  Uint8List putGet(String key, Uint8List bytes);
+
+  /// If this store supports reference counting, decrements it for every key
+  /// in [keys], and evicts entries with the reference count equal zero.
+  void release(Iterable<String> keys);
 }
 
 /// [ByteStore] which stores data only in memory.
 class MemoryByteStore implements ByteStore {
-  final Map<String, Uint8List> _map = {};
+  @visibleForTesting
+  final Map<String, MemoryByteStoreEntry> map = {};
 
   @override
   Uint8List? get(String key) {
-    return _map[key];
+    final entry = map[key];
+    if (entry == null) {
+      return null;
+    }
+
+    entry.refCount++;
+    return entry.bytes;
   }
 
   @override
-  void put(String key, Uint8List bytes) {
-    _map[key] = bytes;
+  Uint8List putGet(String key, Uint8List bytes) {
+    map[key] = MemoryByteStoreEntry._(bytes);
+    return bytes;
+  }
+
+  @override
+  void release(Iterable<String> keys) {
+    for (final key in keys) {
+      final entry = map[key];
+      if (entry != null) {
+        entry.refCount--;
+        if (entry.refCount == 0) {
+          map.remove(key);
+        }
+      }
+    }
+  }
+}
+
+@visibleForTesting
+class MemoryByteStoreEntry {
+  final Uint8List bytes;
+  int refCount = 1;
+
+  MemoryByteStoreEntry._(this.bytes);
+
+  @override
+  String toString() {
+    return '(length: ${bytes.length}, refCount: $refCount)';
   }
 }
 
@@ -65,10 +109,14 @@ class MemoryCachingByteStore implements ByteStore {
   }
 
   @override
-  void put(String key, Uint8List bytes) {
-    _store.put(key, bytes);
+  Uint8List putGet(String key, Uint8List bytes) {
+    _store.putGet(key, bytes);
     _cache.put(key, bytes);
+    return bytes;
   }
+
+  @override
+  void release(Iterable<String> keys) {}
 }
 
 /// [ByteStore] which does not store any data.
@@ -77,5 +125,8 @@ class NullByteStore implements ByteStore {
   Uint8List? get(String key) => null;
 
   @override
-  void put(String key, Uint8List bytes) {}
+  Uint8List putGet(String key, Uint8List bytes) => bytes;
+
+  @override
+  void release(Iterable<String> keys) {}
 }

pkg/analyzer/lib/src/dart/analysis/driver.dart

@@ -1374,7 +1374,7 @@ class AnalysisDriver implements AnalysisDriverGeneric {
           String unitSignature =
               _getResolvedUnitSignature(library.file, unitResult.file);
           String unitKey = _getResolvedUnitKey(unitSignature);
-          _byteStore.put(unitKey, unitBytes);
+          _byteStore.putGet(unitKey, unitBytes);
           if (unitResult.file == file) {
             bytes = unitBytes;
             resolvedUnit = unitResult.unit;
@@ -1855,7 +1855,7 @@ class AnalysisDriver implements AnalysisDriverGeneric {
       String ms = threeDigits(time.millisecond);
       String key = 'exception_${time.year}$m${d}_$h$min${sec}_$ms';
 
-      _byteStore.put(key, bytes);
+      _byteStore.putGet(key, bytes);
       return key;
     } catch (_) {
       return null;

pkg/analyzer/lib/src/dart/analysis/file_byte_store.dart

@@ -44,15 +44,19 @@ class EvictingFileByteStore implements ByteStore {
   Uint8List? get(String key) => _fileByteStore.get(key);
 
   @override
-  void put(String key, Uint8List bytes) {
-    _fileByteStore.put(key, bytes);
+  Uint8List putGet(String key, Uint8List bytes) {
+    _fileByteStore.putGet(key, bytes);
     // Update the current size.
     _bytesWrittenSinceCleanup += bytes.length;
     if (_bytesWrittenSinceCleanup > _maxSizeBytes ~/ 8) {
       _requestCacheCleanUp();
     }
+    return bytes;
   }
 
+  @override
+  void release(Iterable<String> keys) {}
+
   /// If the cache clean up process has not been requested yet, request it.
   Future<void> _requestCacheCleanUp() async {
     if (_cleanUpSendPortShouldBePrepared) {
@@ -176,8 +180,10 @@ class FileByteStore implements ByteStore {
   }
 
   @override
-  void put(String key, Uint8List bytes) {
-    if (!_canShard(key)) return;
+  Uint8List putGet(String key, Uint8List bytes) {
+    if (!_canShard(key)) {
+      return bytes;
+    }
 
     _writeInProgress[key] = bytes;
 
@@ -200,8 +206,13 @@ class FileByteStore implements ByteStore {
         // ignore exceptions
       }
     });
+
+    return bytes;
   }
 
+  @override
+  void release(Iterable<String> keys) {}
+
   String _getShardPath(String key) {
     var shardName = key.substring(0, 2);
     return join(_cachePath, shardName);

pkg/analyzer/lib/src/dart/analysis/file_state.dart

@@ -607,7 +607,7 @@ class FileState {
         unit: unlinkedUnit,
       );
       var bytes = driverUnlinkedUnit.toBytes();
-      _fsState._byteStore.put(_unlinkedKey!, bytes);
+      _fsState._byteStore.putGet(_unlinkedKey!, bytes);
       return driverUnlinkedUnit;
     });
   }

pkg/analyzer/lib/src/dart/analysis/library_context.dart

@@ -215,7 +215,7 @@ class LibraryContext {
         }
 
         resolutionBytes = linkResult.resolutionBytes;
-        byteStore.put(resolutionKey, resolutionBytes);
+        byteStore.putGet(resolutionKey, resolutionBytes);
         bytesPut += resolutionBytes.length;
 
         librariesLinkedTimer.stop();
@@ -241,7 +241,7 @@ class LibraryContext {
             fileSystem: _MacroFileSystem(fileSystemState),
             libraries: macroLibraries,
           );
-          byteStore.put(macroKernelKey, macroKernelBytes);
+          byteStore.putGet(macroKernelKey, macroKernelBytes);
           bytesPut += macroKernelBytes.length;
         } else {
           bytesGet += macroKernelBytes.length;

pkg/analyzer/lib/src/dart/micro/cider_byte_store.dart

@@ -2,86 +2,10 @@
 // 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:typed_data';
+@Deprecated('Use ByteStore directly instead')
+library cider_byte_store;
 
-import 'package:meta/meta.dart';
+import 'package:analyzer/src/dart/analysis/byte_store.dart';
 
-/// Store of bytes associated with string keys and a hash.
-///
-/// Each key must be not longer than 100 characters and consist of only `[a-z]`,
-/// `[0-9]`, `.` and `_` characters. The key cannot be an empty string, the
-/// literal `.`, or contain the sequence `..`.
-///
-/// 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 `null` if the association does not exist.
-  Uint8List? get(String key);
-
-  /// 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 [get] to access data,
-  /// or first [release] it.
-  Uint8List putGet(String key, Uint8List bytes);
-
-  ///  Decrement the reference count for every key in [keys].
-  void release(Iterable<String> keys);
-}
-
-/// [CiderByteStore] that keeps all data in local memory.
-class MemoryCiderByteStore implements CiderByteStore {
-  @visibleForTesting
-  final Map<String, MemoryCiderByteStoreEntry> map = {};
-
-  @override
-  Uint8List? get(String key) {
-    final entry = map[key];
-    if (entry == null) {
-      return null;
-    }
-
-    entry.refCount++;
-    return entry.bytes;
-  }
-
-  @override
-  Uint8List putGet(String key, Uint8List bytes) {
-    if (map.containsKey(key)) {
-      throw StateError('Overwriting is not allowed: $key');
-    }
-
-    map[key] = MemoryCiderByteStoreEntry._(bytes);
-    return bytes;
-  }
-
-  @override
-  void release(Iterable<String> keys) {
-    for (final key in keys) {
-      final entry = map[key];
-      if (entry != null) {
-        entry.refCount--;
-        if (entry.refCount == 0) {
-          map.remove(key);
-        }
-      }
-    }
-  }
-}
-
-@visibleForTesting
-class MemoryCiderByteStoreEntry {
-  final Uint8List bytes;
-  int refCount = 1;
-
-  MemoryCiderByteStoreEntry._(this.bytes);
-
-  @override
-  String toString() {
-    return '(length: ${bytes.length}, refCount: $refCount)';
-  }
-}
+@Deprecated('Use ByteStore directly instead')
+typedef CiderByteStore = ByteStore;

pkg/analyzer/lib/src/dart/micro/library_graph.dart

@@ -14,12 +14,12 @@ import 'package:analyzer/dart/ast/token.dart';
 import 'package:analyzer/dart/element/element.dart';
 import 'package:analyzer/error/listener.dart';
 import 'package:analyzer/file_system/file_system.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/unlinked_api_signature.dart';
 import 'package:analyzer/src/dart/analysis/unlinked_data.dart';
 import 'package:analyzer/src/dart/ast/ast.dart';
-import 'package:analyzer/src/dart/micro/cider_byte_store.dart';
 import 'package:analyzer/src/dart/scanner/reader.dart';
 import 'package:analyzer/src/dart/scanner/scanner.dart';
 import 'package:analyzer/src/generated/parser.dart';
@@ -271,7 +271,7 @@ class FileStateFiles {
 
 class FileSystemState {
   final ResourceProvider _resourceProvider;
-  final CiderByteStore _byteStore;
+  final ByteStore _byteStore;
   final SourceFactory _sourceFactory;
   final Workspace _workspace;
   final Uint32List _linkedSalt;

pkg/analyzer/lib/src/dart/micro/resolve_file.dart

@@ -13,6 +13,7 @@ import 'package:analyzer/file_system/file_system.dart';
 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/byte_store.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;
@@ -22,7 +23,6 @@ import 'package:analyzer/src/dart/analysis/performance_logger.dart';
 import 'package:analyzer/src/dart/analysis/results.dart';
 import 'package:analyzer/src/dart/analysis/search.dart';
 import 'package:analyzer/src/dart/micro/analysis_context.dart';
-import 'package:analyzer/src/dart/micro/cider_byte_store.dart';
 import 'package:analyzer/src/dart/micro/library_analyzer.dart';
 import 'package:analyzer/src/dart/micro/library_graph.dart';
 import 'package:analyzer/src/dart/micro/utils.dart';
@@ -87,7 +87,7 @@ class FileContext {
 class FileResolver {
   final PerformanceLog logger;
   final ResourceProvider resourceProvider;
-  CiderByteStore byteStore;
+  ByteStore byteStore;
   final SourceFactory sourceFactory;
 
   /// A function that returns the digest for a file as a String. The function
@@ -854,7 +854,7 @@ class LibraryContext {
   final FileResolverTestView? testData;
   final PerformanceLog logger;
   final ResourceProvider resourceProvider;
-  final CiderByteStore byteStore;
+  final ByteStore byteStore;
   final MicroContextObjects contextObjects;
 
   Set<LibraryCycle> loadedBundles = Set.identity();

pkg/analyzer/lib/src/services/available_declarations.dart

@@ -1251,7 +1251,7 @@ class _File {
         contentHashBuilder.addString(content);
         contentHashBytes = contentHashBuilder.toByteList();
 
-        tracker._byteStore.put(pathKey, contentHashBytes);
+        tracker._byteStore.putGet(pathKey, contentHashBytes);
       }
 
       contentKey = '${hex.encode(contentHashBytes)}.declarations';
@@ -1844,7 +1844,7 @@ class _File {
           templateNames: templateNames, templateValues: templateValues),
     );
     var bytes = builder.toBuffer();
-    tracker._byteStore.put(contentKey, bytes);
+    tracker._byteStore.putGet(contentKey, bytes);
   }
 
   void _readFileDeclarationsFromBytes(List<int> bytes) {

pkg/analyzer/test/src/dart/analysis/byte_store_test.dart

@@ -29,9 +29,9 @@ class MemoryCachingByteStoreTest {
     cachingStore.get('1');
 
     // Add enough data to the store to force an eviction.
-    cachingStore.put('2', _b(40));
-    cachingStore.put('3', _b(40));
-    cachingStore.put('4', _b(40));
+    cachingStore.putGet('2', _b(40));
+    cachingStore.putGet('3', _b(40));
+    cachingStore.putGet('4', _b(40));
   }
 
   test_get_notFound_retry() {
@@ -43,7 +43,7 @@ class MemoryCachingByteStoreTest {
     expect(cachingStore.get('1'), isNull);
 
     // Add data to the base store, bypassing the caching store.
-    baseStore.put('1', _b(40));
+    baseStore.putGet('1', _b(40));
 
     // Request '1' again.  The previous `null` result should not have been
     // cached.
@@ -55,8 +55,8 @@ class MemoryCachingByteStoreTest {
     var cachingStore = MemoryCachingByteStore(store, 100);
 
     // Keys: [1, 2].
-    cachingStore.put('1', _b(40));
-    cachingStore.put('2', _b(50));
+    cachingStore.putGet('1', _b(40));
+    cachingStore.putGet('2', _b(50));
 
     // Request '1', so now it is the most recently used.
     // Keys: [2, 1].
@@ -64,7 +64,7 @@ class MemoryCachingByteStoreTest {
 
     // 40 + 50 + 30 > 100
     // So, '2' is evicted.
-    cachingStore.put('3', _b(30));
+    cachingStore.putGet('3', _b(30));
     expect(cachingStore.get('1'), hasLength(40));
     expect(cachingStore.get('2'), isNull);
     expect(cachingStore.get('3'), hasLength(30));
@@ -75,14 +75,14 @@ class MemoryCachingByteStoreTest {
     var cachingStore = MemoryCachingByteStore(store, 100);
 
     // 40 + 50 < 100
-    cachingStore.put('1', _b(40));
-    cachingStore.put('2', _b(50));
+    cachingStore.putGet('1', _b(40));
+    cachingStore.putGet('2', _b(50));
     expect(cachingStore.get('1'), hasLength(40));
     expect(cachingStore.get('2'), hasLength(50));
 
     // 40 + 50 + 30 > 100
     // So, '1' is evicted.
-    cachingStore.put('3', _b(30));
+    cachingStore.putGet('3', _b(30));
     expect(cachingStore.get('1'), isNull);
     expect(cachingStore.get('2'), hasLength(50));
     expect(cachingStore.get('3'), hasLength(30));
@@ -93,14 +93,14 @@ class MemoryCachingByteStoreTest {
     var cachingStore = MemoryCachingByteStore(store, 100);
 
     // 10 + 80 < 100
-    cachingStore.put('1', _b(10));
-    cachingStore.put('2', _b(80));
+    cachingStore.putGet('1', _b(10));
+    cachingStore.putGet('2', _b(80));
     expect(cachingStore.get('1'), hasLength(10));
     expect(cachingStore.get('2'), hasLength(80));
 
     // 10 + 80 + 30 > 100
     // So, '1' and '2' are evicted.
-    cachingStore.put('3', _b(30));
+    cachingStore.putGet('3', _b(30));
     expect(cachingStore.get('1'), isNull);
     expect(cachingStore.get('2'), isNull);
     expect(cachingStore.get('3'), hasLength(30));
@@ -114,7 +114,7 @@ class NullByteStoreTest {
 
     expect(store.get('1'), isNull);
 
-    store.put('1', _b(10));
+    store.putGet('1', _b(10));
     expect(store.get('1'), isNull);
   }
 }

pkg/analyzer/test/src/dart/analysis/file_state_test.dart

@@ -2727,7 +2727,7 @@ class C {
     expect(file.unlinked2, isNotNull);
 
     // Make the unlinked unit in the byte store zero-length, damaged.
-    byteStore.put(file.test.unlinkedKey, Uint8List(0));
+    byteStore.putGet(file.test.unlinkedKey, Uint8List(0));
 
     // Refresh should not fail, zero bytes in the store are ignored.
     file.refresh();

pkg/analyzer/test/src/dart/micro/file_resolution.dart

@@ -6,8 +6,8 @@ import 'dart:convert';
 
 import 'package:analyzer/dart/analysis/results.dart';
 import 'package:analyzer/file_system/file_system.dart';
+import 'package:analyzer/src/dart/analysis/byte_store.dart';
 import 'package:analyzer/src/dart/analysis/performance_logger.dart';
-import 'package:analyzer/src/dart/micro/cider_byte_store.dart';
 import 'package:analyzer/src/dart/micro/library_graph.dart';
 import 'package:analyzer/src/dart/micro/resolve_file.dart';
 import 'package:analyzer/src/dart/sdk/sdk.dart';
@@ -29,7 +29,7 @@ import '../resolution/resolution.dart';
 class FileResolutionTest with ResourceProviderMixin, ResolutionTest {
   static final String _testFile = '/workspace/dart/test/lib/test.dart';
 
-  final MemoryCiderByteStore byteStore = MemoryCiderByteStore();
+  final MemoryByteStore byteStore = MemoryByteStore();
 
   final FileResolverTestView testData = FileResolverTestView();
 
@@ -167,7 +167,7 @@ class ResolverStatePrinter {
 
   ResolverStatePrinter(this._resourceProvider, this._sink, this._keyShorter);
 
-  void write(MemoryCiderByteStore byteStore, FileSystemState fileSystemState,
+  void write(MemoryByteStore byteStore, FileSystemState fileSystemState,
       LibraryContext libraryContext, FileResolverTestView testData) {
     _writelnWithIndent('files');
     _withIndent(() {