diff --git a/BitFaster.Caching.UnitTests/Lru/ClassicLruTests.cs b/BitFaster.Caching.UnitTests/Lru/ClassicLruTests.cs index 4791a1d0..def8a864 100644 --- a/BitFaster.Caching.UnitTests/Lru/ClassicLruTests.cs +++ b/BitFaster.Caching.UnitTests/Lru/ClassicLruTests.cs @@ -181,6 +181,17 @@ public void WhenKeyIsRequestedItIsCreatedAndCached() result1.Should().Be(result2); } + + [Fact] + public void WhenKeyIsRequestedWithArgItIsCreatedAndCached() + { + var result1 = lru.GetOrAdd(1, valueFactory.Create, "x"); + var result2 = lru.GetOrAdd(1, valueFactory.Create, "y"); + + valueFactory.timesCalled.Should().Be(1); + result1.Should().Be(result2); + } + [Fact] public async Task WhenKeyIsRequesteItIsCreatedAndCachedAsync() { @@ -191,6 +202,16 @@ public async Task WhenKeyIsRequesteItIsCreatedAndCachedAsync() result1.Should().Be(result2); } + [Fact] + public async Task WhenKeyIsRequestedWithArgItIsCreatedAndCachedAsync() + { + var result1 = await lru.GetOrAddAsync(1, valueFactory.CreateAsync, "x").ConfigureAwait(false); + var result2 = await lru.GetOrAddAsync(1, valueFactory.CreateAsync, "y").ConfigureAwait(false); + + valueFactory.timesCalled.Should().Be(1); + result1.Should().Be(result2); + } + [Fact] public void WhenDifferentKeysAreRequestedValueIsCreatedForEach() { diff --git a/BitFaster.Caching/Lru/ClassicLru.cs b/BitFaster.Caching/Lru/ClassicLru.cs index d709b87e..57f9ebcd 100644 --- a/BitFaster.Caching/Lru/ClassicLru.cs +++ b/BitFaster.Caching/Lru/ClassicLru.cs @@ -113,15 +113,9 @@ public bool TryGet(K key, out V value) return false; } - /// - public V GetOrAdd(K key, Func valueFactory) + private bool TryAdd(K key, V value) { - if (this.TryGet(key, out var value)) - { - return value; - } - - var node = new LinkedListNode(new LruItem(key, valueFactory(key))); + var node = new LinkedListNode(new LruItem(key, value)); if (this.dictionary.TryAdd(key, node)) { @@ -152,57 +146,101 @@ public V GetOrAdd(K key, Func valueFactory) Disposer.Dispose(removed.Value.Value); } - return node.Value.Value; + return true; } - return this.GetOrAdd(key, valueFactory); + return false; } /// - public async ValueTask GetOrAddAsync(K key, Func> valueFactory) + public V GetOrAdd(K key, Func valueFactory) { if (this.TryGet(key, out var value)) { return value; } - var node = new LinkedListNode(new LruItem(key, await valueFactory(key))); + value = valueFactory(key); - if (this.dictionary.TryAdd(key, node)) + if (TryAdd(key, value)) { - LinkedListNode first = null; + return value; + } - lock (this.linkedList) - { - if (linkedList.Count >= capacity) - { - first = linkedList.First; - linkedList.RemoveFirst(); - } + return this.GetOrAdd(key, valueFactory); + } - linkedList.AddLast(node); - } + /// + /// Adds a key/value pair to the cache if the key does not already exist. Returns the new value, or the + /// existing value if the key already exists. + /// + /// The type of an argument to pass into valueFactory. + /// The key of the element to add. + /// The factory function used to generate a value for the key. + /// An argument value to pass into valueFactory. + /// The value for the key. This will be either the existing value for the key if the key is already + /// in the cache, or the new value if the key was not in the cache. + public V GetOrAdd(K key, Func valueFactory, TArg factoryArgument) + { + if (this.TryGet(key, out var value)) + { + return value; + } - // Remove from the dictionary outside the lock. This means that the dictionary at this moment - // contains an item that is not in the linked list. If another thread fetches this item, - // LockAndMoveToEnd will ignore it, since it is detached. This means we potentially 'lose' an - // item just as it was about to move to the back of the LRU list and be preserved. The next request - // for the same key will be a miss. Dictionary and list are eventually consistent. - // However, all operations inside the lock are extremely fast, so contention is minimized. - if (first != null) - { - dictionary.TryRemove(first.Value.Key, out var removed); + value = valueFactory(key, factoryArgument); - Interlocked.Increment(ref this.metrics.evictedCount); - Disposer.Dispose(removed.Value.Value); - } + if (TryAdd(key, value)) + { + return value; + } + + return this.GetOrAdd(key, valueFactory, factoryArgument); + } + + /// + public async ValueTask GetOrAddAsync(K key, Func> valueFactory) + { + if (this.TryGet(key, out var value)) + { + return value; + } + + value = await valueFactory(key); - return node.Value.Value; + if (TryAdd(key, value)) + { + return value; } return await this.GetOrAddAsync(key, valueFactory); } + /// + /// Adds a key/value pair to the cache if the key does not already exist. Returns the new value, or the + /// existing value if the key already exists. + /// + /// The type of an argument to pass into valueFactory. + /// The key of the element to add. + /// The factory function used to asynchronously generate a value for the key. + /// An argument value to pass into valueFactory. + /// A task that represents the asynchronous GetOrAdd operation. + public async ValueTask GetOrAddAsync(K key, Func> valueFactory, TArg factoryArgument) + { + if (this.TryGet(key, out var value)) + { + return value; + } + + value = await valueFactory(key, factoryArgument); + + if (TryAdd(key, value)) + { + return value; + } + + return await this.GetOrAddAsync(key, valueFactory, factoryArgument); + } + /// public bool TryRemove(K key) {