diff --git a/pkg/analyzer/lib/src/dart/analysis/byte_store.dart b/pkg/analyzer/lib/src/dart/analysis/byte_store.dart index 7fa38f7ec95..0c096b376d1 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 @@ class MemoryCachingByteStore implements ByteStore { @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 ca97db67c7c..bddb6faa35f 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 { final int _maxSizeBytes; final int Function(V) _meter; - final _map = {}; + @visibleForTesting + final map = {}; 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 @@ class Cache { void _evict() { if (_currentSizeBytes > _maxSizeBytes) { var keysToRemove = []; - 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 @@ class Cache { } } 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 dc781893d7d..0ffa205bf1c 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 @@ class CiderCachedByteStore implements CiderByteStore { @override CacheData? get(String key, Uint8List signature) { - var entry = _cache.get(key, () => null); + final entry = _cache.get(key); if (entry != null && const ListEquality().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 9a6cac35ac9..0531aa6d9e4 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 @@ main() { }); } -List _b(int length) { - return List.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 @@ class CacheTest { // 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> _newBytesCache(int maxSizeBytes) => - Cache>(maxSizeBytes, (bytes) => bytes.length); - - static List? _noBytes() => null; + Cache _newBytesCache(int maxSizeBytes) => + Cache(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 00000000000..9556ee1ea51 --- /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, {}); +}