From 39e06c434bddeb47e2d47d9f14f1224fc65eaaf1 Mon Sep 17 00:00:00 2001 From: Lin Zhang Date: Thu, 8 Sep 2022 17:48:27 +0800 Subject: [PATCH 1/3] Fix stuck in LruCache after updating items Inside the LruCache, it always create a new link node when doing the []= operator, what if we're doing the "updating" operation? The `_entries` will replace the new value to the same key entry, but the link list will have two nodes with a same key, once the capacity is exhausted, the application will stuck in removing item as the second removing with the same key changes nothing. --- .../lib/src/asset/lru_cache.dart | 4 ++ .../test/asset/lru_cache_test.dart | 52 +++++++++++++++++++ 2 files changed, 56 insertions(+) diff --git a/build_runner_core/lib/src/asset/lru_cache.dart b/build_runner_core/lib/src/asset/lru_cache.dart index b5ff2d2f9..22c1b33c8 100644 --- a/build_runner_core/lib/src/asset/lru_cache.dart +++ b/build_runner_core/lib/src/asset/lru_cache.dart @@ -32,6 +32,10 @@ class LruCache { if (entry.weight > _individualWeightMax) { return; } + + if (_entries.containsKey(key)) { + remove(key); + } _entries[key] = entry; _currentWeightTotal += entry.weight; diff --git a/build_runner_core/test/asset/lru_cache_test.dart b/build_runner_core/test/asset/lru_cache_test.dart index e7af987e5..c8bfe1c38 100644 --- a/build_runner_core/test/asset/lru_cache_test.dart +++ b/build_runner_core/test/asset/lru_cache_test.dart @@ -1,6 +1,7 @@ // Copyright (c) 2018, 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. +@Timeout(Duration(seconds: 10)) import 'package:build_runner_core/src/asset/lru_cache.dart'; import 'package:test/test.dart'; @@ -66,4 +67,55 @@ void main() { expect(cache['3'], null); expect(cache['4'], null); }); + + test('can update values', () { + cache['a'] = 1; + cache['b'] = 2; + expect(cache['a'], 1); + expect(cache['b'], 2); + + cache['a'] = 3; + cache['b'] = 4; + expect(cache['a'], 3); + expect(cache['b'], 4); + }); + + test('removes least recently used or updated items when full', () { + // Populate the cache until full. + var total = 0; + var n = 0; + while (total + maxIndividualWeight <= maxTotalWeight) { + total += maxIndividualWeight; + cache['$n'] = maxIndividualWeight; + n++; + } + // Read all the items in order, the first is now the least recently used. + for (var i = 0; i < n; i++) { + expect(cache['$i'], maxIndividualWeight); + } + + // Update the first item, then the second is now the least recently used. + cache['0'] = maxIndividualWeight; + // Add another item, the second item should now be removed. + cache['${n+1}'] = maxIndividualWeight; + expect(cache['1'], null); + + // Read the first n-1 items, then the latest added item is now the least recently used. + for (var i = 0; i < n; i++) { + if (i != 1) { + expect(cache['$i'], maxIndividualWeight); + } + } + + // Update all the items in order, the last added item should be removed + for (var i = 0; i < n; i++) { + cache['$i'] = maxIndividualWeight; + } + expect(cache['${n}'], null); + + // Check the first n entries shouldn't be removed + for (var i = 0; i < n; i++) { + expect(cache['$i'], maxIndividualWeight); + } + }); } From 6a1f04eaedfbe0498601e9ac76a978e55e025f8b Mon Sep 17 00:00:00 2001 From: Jake Macdonald Date: Thu, 8 Sep 2022 08:34:07 -0700 Subject: [PATCH 2/3] format, update changelog, fix analyzer lint --- build_runner_core/CHANGELOG.md | 1 + build_runner_core/lib/src/asset/lru_cache.dart | 2 +- build_runner_core/test/asset/lru_cache_test.dart | 9 +++++---- 3 files changed, 7 insertions(+), 5 deletions(-) diff --git a/build_runner_core/CHANGELOG.md b/build_runner_core/CHANGELOG.md index 5efd73d38..cf38714f0 100644 --- a/build_runner_core/CHANGELOG.md +++ b/build_runner_core/CHANGELOG.md @@ -1,6 +1,7 @@ ## 7.2.4-dev - Fix some new lints. +- Fix a bug in the LRU cache. ## 7.2.3 diff --git a/build_runner_core/lib/src/asset/lru_cache.dart b/build_runner_core/lib/src/asset/lru_cache.dart index 22c1b33c8..f4aea4f17 100644 --- a/build_runner_core/lib/src/asset/lru_cache.dart +++ b/build_runner_core/lib/src/asset/lru_cache.dart @@ -32,7 +32,7 @@ class LruCache { if (entry.weight > _individualWeightMax) { return; } - + if (_entries.containsKey(key)) { remove(key); } diff --git a/build_runner_core/test/asset/lru_cache_test.dart b/build_runner_core/test/asset/lru_cache_test.dart index c8bfe1c38..8ca9d34b3 100644 --- a/build_runner_core/test/asset/lru_cache_test.dart +++ b/build_runner_core/test/asset/lru_cache_test.dart @@ -97,13 +97,14 @@ void main() { // Update the first item, then the second is now the least recently used. cache['0'] = maxIndividualWeight; // Add another item, the second item should now be removed. - cache['${n+1}'] = maxIndividualWeight; + cache['${n + 1}'] = maxIndividualWeight; expect(cache['1'], null); - // Read the first n-1 items, then the latest added item is now the least recently used. + // Read the first n-1 items, then the latest added item is now the least + // recently used. for (var i = 0; i < n; i++) { if (i != 1) { - expect(cache['$i'], maxIndividualWeight); + expect(cache['$i'], maxIndividualWeight); } } @@ -111,7 +112,7 @@ void main() { for (var i = 0; i < n; i++) { cache['$i'] = maxIndividualWeight; } - expect(cache['${n}'], null); + expect(cache['$n'], null); // Check the first n entries shouldn't be removed for (var i = 0; i < n; i++) { From 9748148d13b27bfe291e1ef8a962e89d92b16463 Mon Sep 17 00:00:00 2001 From: Jake Macdonald Date: Thu, 8 Sep 2022 08:35:38 -0700 Subject: [PATCH 3/3] update to always remove pre-existing keys if present --- build_runner_core/lib/src/asset/lru_cache.dart | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/build_runner_core/lib/src/asset/lru_cache.dart b/build_runner_core/lib/src/asset/lru_cache.dart index f4aea4f17..dae0c6f86 100644 --- a/build_runner_core/lib/src/asset/lru_cache.dart +++ b/build_runner_core/lib/src/asset/lru_cache.dart @@ -27,16 +27,13 @@ class LruCache { } void operator []=(K key, V value) { + remove(key); var entry = _Link(key, value, _computeWeight(value)); // Don't cache at all if above the individual weight max. if (entry.weight > _individualWeightMax) { return; } - if (_entries.containsKey(key)) { - remove(key); - } - _entries[key] = entry; _currentWeightTotal += entry.weight; _promote(entry);