Skip to content

Commit

Permalink
Fix LRU clear when warm/hot items are touched (#453)
Browse files Browse the repository at this point in the history 
* Fix LRU clear when warm/hot items are touched

* add trim test

* special case

* cleanup

---------
  • Loading branch information
@bitfaster
bitfaster authored Nov 7, 2023
1 parent 5275b07 commit 175e9d5
Show file tree
Hide file tree
Showing 2 changed files with 114 additions and 21 deletions.
101 changes: 100 additions & 1 deletion BitFaster.Caching.UnitTests/Lru/ConcurrentLruTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
using Xunit.Abstractions;
using System.Collections.Concurrent;
using System.Reflection;
using System.Runtime.CompilerServices;

namespace BitFaster.Caching.UnitTests.Lru
{
Expand Down Expand Up @@ -875,6 +874,67 @@ public void WhenItemsExistClearRemovesAllItems()
lru.ColdCount.Should().Be(0);
}

// This is a special case:
// Cycle 1: hot => warm
// Cycle 2: warm => warm
// Cycle 3: warm => cold
// Cycle 4: cold => remove
// Cycle 5: cold => remove
[Fact]
public void WhenCacheIsSize3ItemsExistAndItemsAccessedClearRemovesAllItems()
{
lru = new ConcurrentLru<int, string>(3);

lru.AddOrUpdate(1, "1");
lru.AddOrUpdate(2, "1");

lru.TryGet(1, out _);
lru.TryGet(2, out _);

lru.Clear();

lru.Count.Should().Be(0);
}

[Theory]
[InlineData(1)]
[InlineData(2)]
[InlineData(3)]
[InlineData(4)]
[InlineData(5)]
[InlineData(6)]
[InlineData(7)]
[InlineData(8)]
[InlineData(9)]
[InlineData(10)]
public void WhenItemsExistAndItemsAccessedClearRemovesAllItems(int itemCount)
{
// By default capacity is 9. Test all possible states of touched items
// in the cache.

for (int i = 0; i < itemCount; i++)
{
lru.AddOrUpdate(i, "1");
}

// touch n items
for (int i = 0; i < itemCount; i++)
{
lru.TryGet(i, out _);
}

lru.Clear();

this.testOutputHelper.WriteLine("LRU " + string.Join(" ", lru.Keys));

lru.Count.Should().Be(0);

// verify queues are purged
lru.HotCount.Should().Be(0);
lru.WarmCount.Should().Be(0);
lru.ColdCount.Should().Be(0);
}

[Fact]
public void WhenWarmThenClearedIsWarmIsReset()
{
Expand Down Expand Up @@ -1079,6 +1139,45 @@ public void WhenColdItemsAreTouchedTrimRemovesExpectedItemCount(int trimCount, i
this.testOutputHelper.WriteLine("exp " + string.Join(" ", expected));

lru.Keys.Should().BeEquivalentTo(expected);
}

[Theory]
[InlineData(1)]
[InlineData(2)]
[InlineData(3)]
[InlineData(4)]
[InlineData(5)]
[InlineData(6)]
[InlineData(7)]
[InlineData(8)]
[InlineData(9)]
[InlineData(10)]
public void WhenItemsExistAndItemsAccessedTrimRemovesAllItems(int itemCount)
{
// By default capacity is 9. Test all possible states of touched items
// in the cache.

for (int i = 0; i < itemCount; i++)
{
lru.AddOrUpdate(i, "1");
}

// touch n items
for (int i = 0; i < itemCount; i++)
{
lru.TryGet(i, out _);
}

lru.Trim(Math.Min(itemCount, lru.Capacity));

this.testOutputHelper.WriteLine("LRU " + string.Join(" ", lru.Keys));

lru.Count.Should().Be(0);

// verify queues are purged
lru.HotCount.Should().Be(0);
lru.WarmCount.Should().Be(0);
lru.ColdCount.Should().Be(0);
}

[Fact]
Expand Down
34 changes: 14 additions & 20 deletions BitFaster.Caching/Lru/ConcurrentLruCore.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -422,15 +422,7 @@ public void AddOrUpdate(K key, V value)
///<inheritdoc/>
public void Clear()
{
int count = this.Count;

for (int i = 0; i < count; i++)
{
CycleHotUnchecked(ItemRemovedReason.Cleared);
CycleWarmUnchecked(ItemRemovedReason.Cleared);
TryRemoveCold(ItemRemovedReason.Cleared);
}

this.TrimLiveItems(itemsRemoved: 0, this.Count, ItemRemovedReason.Cleared);
Volatile.Write(ref this.isWarm, false);
}

Expand Down Expand Up @@ -458,7 +450,7 @@ public void Trim(int itemCount)
// first scan each queue for discardable items and remove them immediately. Note this can remove > itemCount items.
int itemsRemoved = this.itemPolicy.CanDiscard() ? TrimAllDiscardedItems() : 0;

TrimLiveItems(itemsRemoved, itemCount);
TrimLiveItems(itemsRemoved, itemCount, ItemRemovedReason.Trimmed);
}

private void TrimExpired()
Expand Down Expand Up @@ -507,42 +499,44 @@ void RemoveDiscardableItems(ConcurrentQueue<I> q, ref int queueCounter)
return itemsRemoved;
}

private void TrimLiveItems(int itemsRemoved, int itemCount)
private void TrimLiveItems(int itemsRemoved, int itemCount, ItemRemovedReason reason)
{
// When items are touched, they are moved to warm by cycling. Therefore, to guarantee
// that we can remove itemCount items, we must cycle (2 * capacity.Warm) + capacity.Hot times.
// If clear is called during trimming, it would be possible to get stuck in an infinite
// loop here. Instead quit after n consecutive failed attempts to move warm/hot to cold.
// loop here. The warm + hot limit also guards against this case.
int trimWarmAttempts = 0;
int maxAttempts = this.capacity.Cold + 1;
int maxWarmHotAttempts = (this.capacity.Warm * 2) + this.capacity.Hot;

while (itemsRemoved < itemCount && trimWarmAttempts < maxAttempts)
while (itemsRemoved < itemCount && trimWarmAttempts < maxWarmHotAttempts)
{
if (Volatile.Read(ref this.counter.cold) > 0)
{
if (TryRemoveCold(ItemRemovedReason.Trimmed) == (ItemDestination.Remove, 0))
if (TryRemoveCold(reason) == (ItemDestination.Remove, 0))
{
itemsRemoved++;
trimWarmAttempts = 0;
}

TrimWarmOrHot();
TrimWarmOrHot(reason);
}
else
{
TrimWarmOrHot();
TrimWarmOrHot(reason);
trimWarmAttempts++;
}
}
}

private void TrimWarmOrHot()
private void TrimWarmOrHot(ItemRemovedReason reason)
{
if (Volatile.Read(ref this.counter.warm) > 0)
{
CycleWarmUnchecked(ItemRemovedReason.Trimmed);
CycleWarmUnchecked(reason);
}
else if (Volatile.Read(ref this.counter.hot) > 0)
{
CycleHotUnchecked(ItemRemovedReason.Trimmed);
CycleHotUnchecked(reason);
}
}

Expand Down

0 comments on commit 175e9d5

Please sign in to comment.