Skip to content

Change value factory input argument in IAlternateLookup from TAlternateKey to TKey#752

Merged
bitfaster merged 3 commits intomainfrom
copilot/change-value-factory-input-type
Apr 11, 2026
Merged

Change value factory input argument in IAlternateLookup from TAlternateKey to TKey#752
bitfaster merged 3 commits intomainfrom
copilot/change-value-factory-input-type

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Apr 11, 2026

Summary

Changes the type of the value factory input argument in IAlternateLookup and IAsyncAlternateLookup from TAlternateKey to TKey.

Previously GetOrAdd / GetOrAddAsync passed the alternate key (e.g. ReadOnlySpan<char>) into the value factory. After this change the canonical cache key (TKey, e.g. string) is passed instead. This is more useful since the factory typically needs the strongly-typed key to produce the value, and it also avoids passing a potentially short-lived ref struct into a callback.

Changes

Interfaces

  • IAlternateLookup<TAlternateKey, TKey, TValue>GetOrAdd / GetOrAdd<TArg> factory signatures changed from Func<TAlternateKey, …> to Func<TKey, …>
  • IAsyncAlternateLookup<TAlternateKey, TKey, TValue>GetOrAddAsync / GetOrAddAsync<TArg> factory signatures changed from Func<TAlternateKey, …> to Func<TKey, …>

Implementations – all now pass actualKey (TKey) to the factory instead of key (TAlternateKey):

  • ConcurrentLruCore.cs (AlternateLookup & AsyncAlternateLookup structs)
  • ClassicLru.cs (AlternateLookup struct)
  • ConcurrentLfuCore.cs (AlternateLookup struct)
  • AtomicFactoryCache.cs (AlternateLookup struct)

Tests – updated factory lambdas to use TKey parameter:

  • ConcurrentLruAlternateLookupTests, ClassicLruAlternateLookupTests
  • ConcurrentLruAsyncAlternateLookupTests, ClassicLruAsyncAlternateLookupTests
  • ConcurrentLfuAlternateLookupTests, ConcurrentLfuAsyncAlternateLookupTests
  • AtomicFactoryCacheAlternateLookupTests
  • ConcurrentLruSoakTests

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Updates the alternate-key lookup APIs so GetOrAdd/GetOrAddAsync value factories receive the canonical cache key type (TKey) rather than the alternate key type (TAlternateKey), and propagates that change through implementations and tests.

Changes:

  • Change IAlternateLookup/IAsyncAlternateLookup factory delegate signatures from Func<TAlternateKey,…> to Func<TKey,…>.
  • Update LRU/LFU/Atomic alternate-lookup implementations to pass actualKey (TKey) into the factory.
  • Update unit/soak tests to use the new TKey factory parameter type.

Reviewed changes

Copilot reviewed 14 out of 14 changed files in this pull request and generated 10 comments.

Show a summary per file
File Description
BitFaster.Caching/Lru/ConcurrentLruCore.cs Passes actualKey (K) into sync/async alternate-lookup factories.
BitFaster.Caching/Lru/ClassicLru.cs Passes actualKey (K) into sync/async alternate-lookup factories.
BitFaster.Caching/Lfu/ConcurrentLfuCore.cs Passes actualKey (K) into sync/async alternate-lookup factories.
BitFaster.Caching/IAsyncAlternateLookup.cs Changes async factory delegate types to Func<TKey,…>.
BitFaster.Caching/IAlternateLookup.cs Changes sync factory delegate types to Func<TKey,…>.
BitFaster.Caching/Atomic/AtomicFactoryCache.cs Updates alternate-lookup adapter to accept Func<K,…> factories.
BitFaster.Caching.UnitTests/Lru/ConcurrentLruSoakTests.cs Updates alternate-lookup soak factories to accept string key input.
BitFaster.Caching.UnitTests/Lru/ConcurrentLruAsyncAlternateLookupTests.cs Updates async factory lambdas to use TKey parameter.
BitFaster.Caching.UnitTests/Lru/ConcurrentLruAlternateLookupTests.cs Updates sync factory lambdas to use TKey parameter.
BitFaster.Caching.UnitTests/Lru/ClassicLruAsyncAlternateLookupTests.cs Updates async factory lambdas to use TKey parameter.
BitFaster.Caching.UnitTests/Lru/ClassicLruAlternateLookupTests.cs Updates sync factory lambdas to use TKey parameter.
BitFaster.Caching.UnitTests/Lfu/ConcurrentLfuAsyncAlternateLookupTests.cs Updates async factory lambdas to use TKey parameter.
BitFaster.Caching.UnitTests/Lfu/ConcurrentLfuAlternateLookupTests.cs Updates sync factory lambdas to use TKey parameter.
BitFaster.Caching.UnitTests/Atomic/AtomicFactoryCacheAlternateLookupTests.cs Updates sync factory lambdas to use TKey parameter.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread BitFaster.Caching/IAlternateLookup.cs
Comment thread BitFaster.Caching/IAlternateLookup.cs
Comment thread BitFaster.Caching/IAsyncAlternateLookup.cs
Comment on lines 42 to 46
var result = await alternate.GetOrAddAsync("42".AsSpan(), key =>
{
factoryCalls++;
return Task.FromResult($"value-{key.ToString()}");
return Task.FromResult($"value-{key}");
});
Comment thread BitFaster.Caching.UnitTests/Lru/ClassicLruAlternateLookupTests.cs
Comment on lines 42 to 46
var result = await alternate.GetOrAddAsync("42".AsSpan(), key =>
{
factoryCalls++;
return Task.FromResult($"value-{key.ToString()}");
return Task.FromResult($"value-{key}");
});
Comment on lines 60 to 64
alternate.GetOrAdd(key, key =>
{
factoryCalls++;
return $"value-{key.ToString()}";
return $"value-{key}";
}).Should().Be("value-42");
@bitfaster
Copy link
Copy Markdown
Owner

@copilot fix the build, update the xml doc issues in the code review and ensure all tests pass.

…rom AlternateKey to ActualKey

Agent-Logs-Url: https://github.com/bitfaster/BitFaster.Caching/sessions/880908de-2e9a-47de-a736-2080c1044131

Co-authored-by: bitfaster <12851828+bitfaster@users.noreply.github.com>
Copy link
Copy Markdown
Contributor Author

Copilot AI commented Apr 11, 2026

@copilot fix the build, update the xml doc issues in the code review and ensure all tests pass.

Done in d5c3b16:

  • Updated XML docs for valueFactory in both IAlternateLookup and IAsyncAlternateLookup to explicitly state the factory is invoked with the actual cache key (TKey)
  • Renamed all test methods that said ...UsesAlternateKey... to ...UsesActualKey... across the 7 affected test files
  • Build and all 107 alternate lookup tests pass

@bitfaster
Copy link
Copy Markdown
Owner

@copilot fix the benchmark so it can build and run.

…har>, int> to Func<string, int>

Agent-Logs-Url: https://github.com/bitfaster/BitFaster.Caching/sessions/8e228955-be34-438b-b930-aba65302bb90

Co-authored-by: bitfaster <12851828+bitfaster@users.noreply.github.com>
Copy link
Copy Markdown
Contributor Author

Copilot AI commented Apr 11, 2026

@copilot fix the benchmark so it can build and run.

Fixed in de68646. LruJustGetOrAddAlternate.cs and LfuJustGetOrAddAlternate.cs both had Func<ReadOnlySpan<char>, int> locals — updated to Func<string, int> to match the new factory signature. Benchmark project now builds cleanly on net9.0.

@coveralls
Copy link
Copy Markdown

Coverage Status

coverage: 99.02%. first build — copilot/change-value-factory-input-type into main

@bitfaster bitfaster marked this pull request as ready for review April 11, 2026 22:07
@bitfaster bitfaster merged commit 089a440 into main Apr 11, 2026
16 checks passed
@bitfaster bitfaster deleted the copilot/change-value-factory-input-type branch April 11, 2026 22:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants