Skip to content

Commit

Permalink
Fix stuck in LruCache after updating items
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
grootzhang committed Sep 8, 2022
1 parent 9d607d5 commit dff50b1
Show file tree
Hide file tree
Showing 2 changed files with 56 additions and 0 deletions.
4 changes: 4 additions & 0 deletions build_runner_core/lib/src/asset/lru_cache.dart
Expand Up @@ -32,6 +32,10 @@ class LruCache<K, V> {
if (entry.weight > _individualWeightMax) {
return;
}

if (_entries.containsKey(key)) {
remove(key);
}

_entries[key] = entry;
_currentWeightTotal += entry.weight;
Expand Down
52 changes: 52 additions & 0 deletions 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';
Expand Down Expand Up @@ -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);
}
});
}

0 comments on commit dff50b1

Please sign in to comment.