- 
                Notifications
    You must be signed in to change notification settings 
- Fork 2.1k
          Replace LRU with implementation based on BitFaster.Caching ConcurrentLru
          #9530
        
          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
Conversation
341ae2d    to
    1d25967      
    Compare
  
    1d25967    to
    7c23def      
    Compare
  
    There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR replaces the existing custom LRU cache with a BitFaster.Caching–based ConcurrentLruCache and updates all consumers accordingly.
- Removes the legacy LRU<TKey,TValue>implementation and ports/simplifies BitFaster’sConcurrentLruinternals underOrleans.Caching.Internal.
- Updates GrainDirectory caches (LRU and Adaptive) and messaging codecs to use ConcurrentLruCache.
- Changes the default directory caching policy from Adaptive to LRU.
Reviewed Changes
Copilot reviewed 24 out of 24 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description | 
|---|---|
| src/Orleans.Runtime/Utilities/StripedMpscBuffer.cs | Updated attribution comment and made local Paddingclass static | 
| src/Orleans.Runtime/GrainDirectory/LruGrainDirectoryCache.cs | New LruGrainDirectoryCacheusingConcurrentLruCache | 
| src/Orleans.Runtime/GrainDirectory/LRUBasedGrainDirectoryCache.cs | Removed legacy LRU-based cache class | 
| src/Orleans.Runtime/GrainDirectory/GrainDirectoryCacheFactory.cs | Switched factory to LruGrainDirectoryCache | 
| src/Orleans.Runtime/GrainDirectory/AdaptiveGrainDirectoryCache.cs | Migrated to ConcurrentLruCacheand updated method calls | 
| src/Orleans.Runtime/Configuration/Options/GrainDirectoryOptions.cs | Changed default caching strategy to LRU and formatted default values | 
| src/Orleans.Core/Utils/LRU.cs | Removed old LRU implementation | 
| src/Orleans.Core/Messaging/CachingSiloAddressCodec.cs | Switched shared cache to ConcurrentLruCacheand fixed constructor | 
| src/Orleans.Core/Messaging/CachingIdSpanCodec.cs | Switched shared cache and corrected lambda parameter order | 
| src/Orleans.Core/Caching/Internal/TypeProps.cs | Added atomic‐write helper | 
| src/Orleans.Core/Caching/Internal/Striped64.cs | Added internal striped concurrency helper | 
| src/Orleans.Core/Caching/Internal/Padding.cs | Added cache‐line padding constants | 
| src/Orleans.Core/Caching/Internal/PaddedQueueCount.cs | Added padded queue counts | 
| src/Orleans.Core/Caching/Internal/PaddedLong.cs | Added padded long wrapper | 
| src/Orleans.Core/Caching/Internal/ICacheMetrics.cs | Added cache metrics interface | 
| src/Orleans.Core/Caching/Internal/ICache.cs | Added generic cache interface | 
| src/Orleans.Core/Caching/Internal/Counter.cs | Added high-throughput counter | 
| src/Orleans.Core/Caching/Internal/ConcurrentDictionarySize.cs | Added concurrent dictionary sizing helper | 
| src/Orleans.Core/Caching/Internal/CapacityPartition.cs | Added capacity partitioning scheme | 
| src/Orleans.Core/Caching/Internal/CacheDebugView.cs | Added debug view for caches | 
Comments suppressed due to low confidence (2)
src/Orleans.Runtime.GrainDirectory/AdaptiveGrainDirectoryCache.cs:45
- [nitpick] The predicate is named ActivationAddressesMatcheshere butActivationAddressesMatchinLruGrainDirectoryCache. Consider unifying the naming for consistency.
private static readonly Func<GrainDirectoryCacheEntry, GrainAddress, bool> ActivationAddressesMatches = (entry, addr) => GrainAddress.MatchesGrainIdAndSilo(addr, entry.Address);
src/Orleans.Core/Messaging/CachingSiloAddressCodec.cs:17
- The capacity was reduced from 128_000 in the previous LRU to 1_024 here—verify that this smaller cache size is intentional and sufficient for your workload.
internal static ConcurrentLruCache<SiloAddress, (SiloAddress Value, byte[] Encoded)> SharedCache { get; } = new(capacity: 1024);
175b5d2    to
    8c531c2      
    Compare
  
    There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
This PR replaces the existing LRU implementation with an implementation based on BitFaster.Caching's
ConcurrentLru. The code, including tests, was ported and simplified to remove functionality which Orleans does not leverage.Here is a description of the ConcurrentLru implementation from the source:
Ideally, we would have referenced RCache (whatever it ended up being called) from Microsoft.Extensions.*, but there hasn't been progress there.
This PR also changes the default Directory Cache policy from Adaptive to LRU. We anticipate removing the Adaptive policy in the future.
Fixes #8736
Microsoft Reviewers: Open in CodeFlow