Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ConcurrentLfu.Clear() polluted by removed items #401

Merged
merged 2 commits into from
Oct 2, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
63 changes: 56 additions & 7 deletions BitFaster.Caching.UnitTests/Lfu/ConcurrentLfuTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -56,8 +56,8 @@
[Fact]
public async Task WhenKeyIsRequesteItIsCreatedAndCachedAsync()
{
var result1 = await cache.GetOrAddAsync(1, valueFactory.CreateAsync).ConfigureAwait(false);

Check warning on line 59 in BitFaster.Caching.UnitTests/Lfu/ConcurrentLfuTests.cs

View workflow job for this annotation

GitHub Actions / mac

Test methods should not call ConfigureAwait(), as it may bypass parallelization limits. (https://xunit.net/xunit.analyzers/rules/xUnit1030)

Check warning on line 59 in BitFaster.Caching.UnitTests/Lfu/ConcurrentLfuTests.cs

View workflow job for this annotation

GitHub Actions / infer

Test methods should not call ConfigureAwait(), as it may bypass parallelization limits.

Check warning on line 59 in BitFaster.Caching.UnitTests/Lfu/ConcurrentLfuTests.cs

View workflow job for this annotation

GitHub Actions / win

Test methods should not call ConfigureAwait(), as it may bypass parallelization limits.

Check warning on line 59 in BitFaster.Caching.UnitTests/Lfu/ConcurrentLfuTests.cs

View workflow job for this annotation

GitHub Actions / bench

Test methods should not call ConfigureAwait(), as it may bypass parallelization limits.
var result2 = await cache.GetOrAddAsync(1, valueFactory.CreateAsync).ConfigureAwait(false);

Check warning on line 60 in BitFaster.Caching.UnitTests/Lfu/ConcurrentLfuTests.cs

View workflow job for this annotation

GitHub Actions / mac

Test methods should not call ConfigureAwait(), as it may bypass parallelization limits. (https://xunit.net/xunit.analyzers/rules/xUnit1030)

Check warning on line 60 in BitFaster.Caching.UnitTests/Lfu/ConcurrentLfuTests.cs

View workflow job for this annotation

GitHub Actions / infer

Test methods should not call ConfigureAwait(), as it may bypass parallelization limits.

Check warning on line 60 in BitFaster.Caching.UnitTests/Lfu/ConcurrentLfuTests.cs

View workflow job for this annotation

GitHub Actions / win

Test methods should not call ConfigureAwait(), as it may bypass parallelization limits.

Check warning on line 60 in BitFaster.Caching.UnitTests/Lfu/ConcurrentLfuTests.cs

View workflow job for this annotation

GitHub Actions / bench

Test methods should not call ConfigureAwait(), as it may bypass parallelization limits.

valueFactory.timesCalled.Should().Be(1);
result1.Should().Be(result2);
Expand All @@ -66,8 +66,8 @@
[Fact]
public async Task WhenKeyIsRequestedWithArgItIsCreatedAndCachedAsync()
{
var result1 = await cache.GetOrAddAsync(1, valueFactory.CreateAsync, 9).ConfigureAwait(false);

Check warning on line 69 in BitFaster.Caching.UnitTests/Lfu/ConcurrentLfuTests.cs

View workflow job for this annotation

GitHub Actions / mac

Test methods should not call ConfigureAwait(), as it may bypass parallelization limits. (https://xunit.net/xunit.analyzers/rules/xUnit1030)

Check warning on line 69 in BitFaster.Caching.UnitTests/Lfu/ConcurrentLfuTests.cs

View workflow job for this annotation

GitHub Actions / infer

Test methods should not call ConfigureAwait(), as it may bypass parallelization limits.

Check warning on line 69 in BitFaster.Caching.UnitTests/Lfu/ConcurrentLfuTests.cs

View workflow job for this annotation

GitHub Actions / win

Test methods should not call ConfigureAwait(), as it may bypass parallelization limits.

Check warning on line 69 in BitFaster.Caching.UnitTests/Lfu/ConcurrentLfuTests.cs

View workflow job for this annotation

GitHub Actions / bench

Test methods should not call ConfigureAwait(), as it may bypass parallelization limits.
var result2 = await cache.GetOrAddAsync(1, valueFactory.CreateAsync, 17).ConfigureAwait(false);

Check warning on line 70 in BitFaster.Caching.UnitTests/Lfu/ConcurrentLfuTests.cs

View workflow job for this annotation

GitHub Actions / mac

Test methods should not call ConfigureAwait(), as it may bypass parallelization limits. (https://xunit.net/xunit.analyzers/rules/xUnit1030)

Check warning on line 70 in BitFaster.Caching.UnitTests/Lfu/ConcurrentLfuTests.cs

View workflow job for this annotation

GitHub Actions / infer

Test methods should not call ConfigureAwait(), as it may bypass parallelization limits.

Check warning on line 70 in BitFaster.Caching.UnitTests/Lfu/ConcurrentLfuTests.cs

View workflow job for this annotation

GitHub Actions / win

Test methods should not call ConfigureAwait(), as it may bypass parallelization limits.

Check warning on line 70 in BitFaster.Caching.UnitTests/Lfu/ConcurrentLfuTests.cs

View workflow job for this annotation

GitHub Actions / bench

Test methods should not call ConfigureAwait(), as it may bypass parallelization limits.

valueFactory.timesCalled.Should().Be(1);
result1.Should().Be(result2);
Expand Down Expand Up @@ -677,15 +677,64 @@
[Fact]
public void WhenClearedCacheIsEmpty()
{
cache.GetOrAdd(1, k => k);
cache.GetOrAdd(2, k => k);
cache.DoMaintenance();
int key = 0;
const int count = 20;

for (int j = 0; j < 100; j++)
{
cache = new ConcurrentLfu<int, int>(1, count, new NullScheduler(), EqualityComparer<int>.Default);

for (int i = 0; i < count; i++)
{
cache.GetOrAdd(key++, k => k);
}

cache.Clear();
cache.DoMaintenance();
cache.Count.Should().Be(count);

cache.Count.Should().Be(0);
cache.TryGet(1, out var _).Should().BeFalse();
cache.Clear();

cache.Count.Should().Be(0);
cache.TryGet(1, out var _).Should().BeFalse();
}
}

[Fact]
public async Task TestClear()
bitfaster marked this conversation as resolved.
Show resolved Hide resolved
{
var cache = new ConcurrentLfuBuilder<string, string>()
.WithScheduler(new BackgroundThreadScheduler())
.WithCapacity(40)
.WithAtomicGetOrAdd()
.AsAsyncCache()
.Build();

var emptyCount = 0;
for (var a = 0; a < 200; a++)
{
for (var i = 0; i < 10; i++)
{
var str = "a" + i;
await getOrAdd(str);
}

cache.Clear();
//cache.Clear(); // attempt to trigger maintenance after previous clear operation

if (cache.Count != 0)
{
emptyCount += cache.Count;
emptyCount += cache.Count;
}


}

emptyCount.Should().Be(0);

async Task getOrAdd(string key)
{
await cache.GetOrAddAsync(key, Task.FromResult);
}
}

[Fact]
Expand Down
7 changes: 6 additions & 1 deletion BitFaster.Caching/Lfu/ConcurrentLfu.cs
Original file line number Diff line number Diff line change
Expand Up @@ -426,7 +426,12 @@ private static void TakeCandidatesInLruOrder(LfuNodeList<K, V> lru, List<LfuNode

while (candidates.Count < itemCount && curr != null)
{
candidates.Add(curr);
// LRUs can contain items that are already removed, skip those
if (!curr.WasRemoved)
{
candidates.Add(curr);
}

curr = curr.Next;
}
}
Expand Down
Loading