Make Cache.get() just return the value.
Change-Id: I816a1c28569011215ad6cba9a7559513a7900386
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/246880
Commit-Queue: Konstantin Shcheglov <scheglov@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
diff --git a/pkg/analyzer/lib/src/dart/analysis/byte_store.dart b/pkg/analyzer/lib/src/dart/analysis/byte_store.dart
index 7fa38f7..0c096b3 100644
--- a/pkg/analyzer/lib/src/dart/analysis/byte_store.dart
+++ b/pkg/analyzer/lib/src/dart/analysis/byte_store.dart
@@ -50,7 +50,18 @@
@override
Uint8List? get(String key) {
- return _cache.get(key, () => _store.get(key));
+ final cached = _cache.get(key);
+ if (cached != null) {
+ return cached;
+ }
+
+ final fromStore = _store.get(key);
+ if (fromStore != null) {
+ _cache.put(key, fromStore);
+ return fromStore;
+ }
+
+ return null;
}
@override
diff --git a/pkg/analyzer/lib/src/dart/analysis/cache.dart b/pkg/analyzer/lib/src/dart/analysis/cache.dart
index ca97db6..bddb6fa 100644
--- a/pkg/analyzer/lib/src/dart/analysis/cache.dart
+++ b/pkg/analyzer/lib/src/dart/analysis/cache.dart
@@ -2,37 +2,33 @@
// 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 'package:meta/meta.dart';
+
/// LRU cache of objects.
class Cache<K, V> {
final int _maxSizeBytes;
final int Function(V) _meter;
- final _map = <K, V>{};
+ @visibleForTesting
+ final map = <K, V>{};
int _currentSizeBytes = 0;
Cache(this._maxSizeBytes, this._meter);
- V? get(K key, V? Function() getNotCached) {
- V? value = _map.remove(key);
- if (value == null) {
- value = getNotCached();
- if (value != null) {
- _map[key] = value;
- _currentSizeBytes += _meter(value);
- _evict();
- }
- } else {
- _map[key] = value;
+ V? get(K key) {
+ final value = map.remove(key);
+ if (value != null) {
+ map[key] = value;
}
return value;
}
void put(K key, V value) {
- V? oldValue = _map[key];
+ V? oldValue = map[key];
if (oldValue != null) {
_currentSizeBytes -= _meter(oldValue);
}
- _map[key] = value;
+ map[key] = value;
_currentSizeBytes += _meter(value);
_evict();
}
@@ -40,7 +36,7 @@
void _evict() {
if (_currentSizeBytes > _maxSizeBytes) {
var keysToRemove = <K>[];
- for (var entry in _map.entries) {
+ for (var entry in map.entries) {
keysToRemove.add(entry.key);
_currentSizeBytes -= _meter(entry.value);
if (_currentSizeBytes <= _maxSizeBytes) {
@@ -48,7 +44,7 @@
}
}
for (var key in keysToRemove) {
- _map.remove(key);
+ map.remove(key);
}
}
}
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 dc78189..0ffa205 100644
--- a/pkg/analyzer/lib/src/dart/micro/cider_byte_store.dart
+++ b/pkg/analyzer/lib/src/dart/micro/cider_byte_store.dart
@@ -54,7 +54,7 @@
@override
CacheData? get(String key, Uint8List signature) {
- var entry = _cache.get(key, () => null);
+ final entry = _cache.get(key);
if (entry != null &&
const ListEquality<int>().equals(entry.signature, signature)) {
diff --git a/pkg/analyzer/test/src/dart/analysis/cache_test.dart b/pkg/analyzer/test/src/dart/analysis/cache_test.dart
index 9a6cac3..0531aa6 100644
--- a/pkg/analyzer/test/src/dart/analysis/cache_test.dart
+++ b/pkg/analyzer/test/src/dart/analysis/cache_test.dart
@@ -2,7 +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';
+
import 'package:analyzer/src/dart/analysis/cache.dart';
+import 'package:analyzer_utilities/testing/map_entry_matcher.dart';
import 'package:test/test.dart';
import 'package:test_reflective_loader/test_reflective_loader.dart';
@@ -12,52 +15,47 @@
});
}
-List<int> _b(int length) {
- return List<int>.filled(length, 0);
+Uint8List _b(int length) {
+ return Uint8List(length);
}
@reflectiveTest
class CacheTest {
- test_get_notFound_evict() {
- var cache = _newBytesCache(100);
+ test_get() {
+ final cache = _newBytesCache(100);
- // Request '1'. Nothing found.
- expect(cache.get('1', _noBytes), isNull);
+ expect(cache.get('1'), isNull);
+ expect(cache.map.entries, isEmpty);
- // Add enough data to the store to force an eviction.
- cache.put('2', _b(40));
- cache.put('3', _b(40));
- cache.put('4', _b(40));
+ cache.put('1', _b(10));
+ expect(cache.map.entries, [
+ isMapEntry('1', hasLength(10)),
+ ]);
+
+ expect(cache.get('1'), hasLength(10));
}
- test_get_notFound_retry() {
- var cache = _newBytesCache(100);
+ test_get_reorders() {
+ final cache = _newBytesCache(100);
- // Request '1'. Nothing found.
- expect(cache.get('1', _noBytes), isNull);
+ cache.put('1', _b(1));
+ cache.put('2', _b(2));
+ cache.put('3', _b(3));
+ cache.put('4', _b(4));
+ expect(cache.map.entries, [
+ isMapEntry('1', hasLength(1)),
+ isMapEntry('2', hasLength(2)),
+ isMapEntry('3', hasLength(3)),
+ isMapEntry('4', hasLength(4)),
+ ]);
- // Request '1' again.
- // The previous `null` result should not have been cached.
- expect(cache.get('1', () => _b(40)), isNotNull);
- }
-
- test_get_put_evict() {
- var cache = _newBytesCache(100);
-
- // Keys: [1, 2].
- cache.put('1', _b(40));
- cache.put('2', _b(50));
-
- // Request '1', so now it is the most recently used.
- // Keys: [2, 1].
- cache.get('1', _noBytes);
-
- // 40 + 50 + 30 > 100
- // So, '2' is evicted.
- cache.put('3', _b(30));
- expect(cache.get('1', _noBytes), hasLength(40));
- expect(cache.get('2', _noBytes), isNull);
- expect(cache.get('3', _noBytes), hasLength(30));
+ expect(cache.get('2'), hasLength(2));
+ expect(cache.map.entries, [
+ isMapEntry('1', hasLength(1)),
+ isMapEntry('3', hasLength(3)),
+ isMapEntry('4', hasLength(4)),
+ isMapEntry('2', hasLength(2)),
+ ]);
}
test_put_evict_first() {
@@ -66,36 +64,39 @@
// 40 + 50 < 100
cache.put('1', _b(40));
cache.put('2', _b(50));
- expect(cache.get('1', _noBytes), hasLength(40));
- expect(cache.get('2', _noBytes), hasLength(50));
+ expect(cache.map.entries, [
+ isMapEntry('1', hasLength(40)),
+ isMapEntry('2', hasLength(50)),
+ ]);
// 40 + 50 + 30 > 100
// So, '1' is evicted.
cache.put('3', _b(30));
- expect(cache.get('1', _noBytes), isNull);
- expect(cache.get('2', _noBytes), hasLength(50));
- expect(cache.get('3', _noBytes), hasLength(30));
+ expect(cache.map.entries, [
+ isMapEntry('2', hasLength(50)),
+ isMapEntry('3', hasLength(30)),
+ ]);
}
- test_put_evict_firstAndSecond() {
+ test_put_evict_firstSecond() {
var cache = _newBytesCache(100);
// 10 + 80 < 100
cache.put('1', _b(10));
cache.put('2', _b(80));
- expect(cache.get('1', _noBytes), hasLength(10));
- expect(cache.get('2', _noBytes), hasLength(80));
+ expect(cache.map.entries, [
+ isMapEntry('1', hasLength(10)),
+ isMapEntry('2', hasLength(80)),
+ ]);
// 10 + 80 + 30 > 100
// So, '1' and '2' are evicted.
cache.put('3', _b(30));
- expect(cache.get('1', _noBytes), isNull);
- expect(cache.get('2', _noBytes), isNull);
- expect(cache.get('3', _noBytes), hasLength(30));
+ expect(cache.map.entries, [
+ isMapEntry('3', hasLength(30)),
+ ]);
}
- Cache<String, List<int>> _newBytesCache(int maxSizeBytes) =>
- Cache<String, List<int>>(maxSizeBytes, (bytes) => bytes.length);
-
- static List<int>? _noBytes() => null;
+ Cache<String, Uint8List> _newBytesCache(int maxSizeBytes) =>
+ Cache<String, Uint8List>(maxSizeBytes, (bytes) => bytes.length);
}
diff --git a/pkg/analyzer_utilities/lib/testing/map_entry_matcher.dart b/pkg/analyzer_utilities/lib/testing/map_entry_matcher.dart
new file mode 100644
index 0000000..9556ee1
--- /dev/null
+++ b/pkg/analyzer_utilities/lib/testing/map_entry_matcher.dart
@@ -0,0 +1,31 @@
+// Copyright (c) 2022, 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 'package:test/test.dart';
+
+/// Matches a [MapEntry] with matching [MapEntry.key] and [MapEntry.value].
+MapEntryMatcher isMapEntry(key, value) => MapEntryMatcher(key, value);
+
+class MapEntryMatcher extends Matcher {
+ final Matcher keyMatcher;
+ final Matcher valueMatcher;
+
+ MapEntryMatcher(key, value)
+ : keyMatcher = wrapMatcher(key),
+ valueMatcher = wrapMatcher(value);
+
+ @override
+ Description describe(Description description) => description
+ .add('MapEntry(key: ')
+ .addDescriptionOf(keyMatcher)
+ .add(', value: ')
+ .addDescriptionOf(valueMatcher)
+ .add(')');
+
+ @override
+ bool matches(item, Map matchState) =>
+ item is MapEntry &&
+ keyMatcher.matches(item.key, {}) &&
+ valueMatcher.matches(item.value, {});
+}
