Conversation
CoPilot session
Optimizes GetAllAsync in HybridCacheClient by parallelizing local cache lookups and distributed cache population. It also reduces network calls to the distributed cache by fetching all expirations in a single bulk operation. This addresses performance issues related to excessive round trips and improves overall efficiency. CoPilot session
Optimizes GetAllExpirationAsync in HybridCacheClient and InMemoryCacheClient by first checking the local cache and then retrieving any missed keys from the distributed cache to reduce distributed cache calls. Adds null checks and optimizes the handling of empty key lists to improve efficiency and prevent errors.
Ensures that setting expirations for multiple cache keys correctly publishes invalidation messages and handles empty expiration dictionaries efficiently. This prevents unnecessary message bus calls when no expirations are set, and guarantees invalidation messages are sent with the correct keys after setting the expirations on the distributed cache.
Optimizes the `GetAllAsync` method in `HybridCacheClient` to reduce latency and improve performance. The previous implementation used parallel processing and thread-safe collections which introduced unnecessary overhead. This commit refactors the method to leverage bulk operations and minimize locking, resulting in more efficient cache retrieval. It now retrieves all local cache values at once, then fetches missing keys from the distributed cache, and finally populates the local cache with the distributed results. This reduces the number of network calls and improves overall throughput.
Updates the documentation to better explain when to use context7. Specifically, it clarifies that context7 should be used when code generation, setup/configuration steps, or library/API documentation is needed.
Addresses inconsistencies in cache key validation by enforcing ArgumentException instead of ArgumentNullException for empty strings in RemoveIfEqualAsync. Simplifies test logic by removing redundant ArgumentNullException checks for empty strings in GetListAsync and ListAddAsync. Adjusts test parameters in RemoveByPrefixAsync to use a smaller value (9999) instead of (10000). This likely addresses edge case issues or performance considerations in the test environment.
Replaces multiple null checks with ArgumentNullException.ThrowIfNull for improved code clarity and performance. Also, replaces manual range checks with ArgumentOutOfRangeException.ThrowIfLessThan. Adds Distinct to keys in RemoveAllAsync to ensure no duplicate keys.
Improves the ScopedCacheClient by adding argument validation to prevent null or empty keys and values. This ensures data integrity and prevents unexpected errors when interacting with the cache.
Optimizes the GetAllAsync method in HybridCacheClient to improve performance by reducing unnecessary iterations and array conversions. It now iterates through the initial collection of keys only once and avoids converting to an array unless necessary. Additionally, it streamlines the process of identifying missed keys and fetching them from the distributed cache.
Improves cache performance analysis by adding a new benchmark to measure SetAllAsync/GetAllAsync throughput with a large number of keys. This helps to evaluate bulk insert and retrieval performance and validates data correctness under high load. Also, it updates existing benchmarks to report operations per second.
There was a problem hiding this comment.
Pull request overview
This PR adds bulk expiration management methods (GetAllExpirationsAsync and SetAllExpirationsAsync) to the Foundatio caching layer, improving efficiency when working with expiration times for multiple keys. The changes also introduce breaking changes to argument validation, refactor test organization into partial classes, and improve test coverage with more descriptive naming.
Key Changes:
- Added
GetAllExpirationsAsyncandSetAllExpirationsAsyncmethods toICacheClientinterface and all implementations - Standardized argument validation using
ArgumentException.ThrowIfNullOrEmptyandArgumentNullException.ThrowIfNull - Refactored tests into partial classes organized by functionality
- Updated test naming to be more descriptive (e.g.,
CanGetAllAsync→GetAllAsync_WithExistingKeys_ReturnsAllValues) - Cleaned up whitespace (removed BOM markers and extra blank lines)
Reviewed changes
Copilot reviewed 43 out of 43 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
src/Foundatio/Caching/ICacheClient.cs |
Added two new interface methods for bulk expiration operations |
src/Foundatio/Caching/InMemoryCacheClient.cs |
Implemented bulk expiration methods with proper validation and optimized bulk operations in GetAllAsync |
src/Foundatio/Caching/HybridCacheClient.cs |
Implemented bulk expiration methods and optimized GetAllAsync to use single bulk expiration call |
src/Foundatio/Caching/HybridAwareCacheClient.cs |
Implemented bulk expiration methods with cache invalidation messaging |
src/Foundatio/Caching/ScopedCacheClient.cs |
Implemented bulk expiration methods with scope key translation and fixed key validation in GetUnscopedCacheKeys |
src/Foundatio/Caching/NullCacheClient.cs |
Implemented no-op bulk expiration methods |
tests/* |
Massive test refactoring with improved naming and organization |
| Whitespace cleanup | Removed BOM markers and extra blank lines across multiple files |
src/Foundatio.TestHarness/Caching/CacheClientTestsBase.GetAllExpirationAsync.cs
Outdated
Show resolved
Hide resolved
src/Foundatio.TestHarness/Caching/CacheClientTestsBase.GetAllExpirationAsync.cs
Outdated
Show resolved
Hide resolved
src/Foundatio.TestHarness/Caching/CacheClientTestsBase.GetAllExpirationAsync.cs
Outdated
Show resolved
Hide resolved
src/Foundatio.TestHarness/Caching/CacheClientTestsBase.GetAllExpirationAsync.cs
Outdated
Show resolved
Hide resolved
src/Foundatio.TestHarness/Caching/CacheClientTestsBase.GetExpirationAsync.cs
Fixed
Show fixed
Hide fixed
src/Foundatio.TestHarness/Caching/CacheClientTestsBase.GetExpirationAsync.cs
Fixed
Show fixed
Hide fixed
src/Foundatio.TestHarness/Caching/CacheClientTestsBase.GetExpirationAsync.cs
Fixed
Show fixed
Hide fixed
src/Foundatio.TestHarness/Caching/CacheClientTestsBase.GetExpirationAsync.cs
Fixed
Show fixed
Hide fixed
src/Foundatio.TestHarness/Caching/CacheClientTestsBase.GetAllExpirationAsync.cs
Fixed
Show fixed
Hide fixed
src/Foundatio.TestHarness/Caching/CacheClientTestsBase.SetExpirationAsync.cs
Fixed
Show fixed
Hide fixed
src/Foundatio.TestHarness/Caching/CacheClientTestsBase.SetExpirationAsync.cs
Fixed
Show fixed
Hide fixed
src/Foundatio.TestHarness/Caching/CacheClientTestsBase.SetAllExpiration.cs
Fixed
Show fixed
Hide fixed
src/Foundatio.TestHarness/Caching/CacheClientTestsBase.SetAllExpiration.cs
Fixed
Show fixed
Hide fixed
…xpirationAsync.cs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
src/Foundatio.TestHarness/Caching/CacheClientTestsBase.GetAllExpirationAsync.cs
Fixed
Show fixed
Hide fixed
src/Foundatio.TestHarness/Caching/CacheClientTestsBase.GetAllExpirationAsync.cs
Fixed
Show fixed
Hide fixed
src/Foundatio.TestHarness/Caching/CacheClientTestsBase.GetAllExpirationAsync.cs
Fixed
Show fixed
Hide fixed
| return base.AddAsync_WithNullKey_ThrowsArgumentNullException(); | ||
| } [Fact] |
There was a problem hiding this comment.
Missing newline between method declarations. There should be a blank line between AddAsync_WithNullKey_ThrowsArgumentNullException() at line 59 and the [Fact] attribute at line 60.
| return base.ExistsAsync_WithScopedCache_ChecksOnlyWithinScope(); | ||
| } [Fact] |
There was a problem hiding this comment.
Missing newline between method declarations. There should be a blank line between ExistsAsync_WithScopedCache_ChecksOnlyWithinScope() at line 125 and the [Fact] attribute at line 126.
Updates RemoveAsync to return false if the entry was expired, consistent with Redis behavior.
Updates the name of the ExistsAsync test method to more accurately reflect its purpose and the variety of scenarios it covers.
src/Foundatio.TestHarness/Caching/CacheClientTestsBase.GetExpirationAsync.cs
Fixed
Show fixed
Hide fixed
src/Foundatio.TestHarness/Caching/CacheClientTestsBase.GetAllExpirationAsync.cs
Fixed
Show fixed
Hide fixed
src/Foundatio.TestHarness/Caching/CacheClientTestsBase.SetAllExpiration.cs
Fixed
Show fixed
Hide fixed
src/Foundatio.TestHarness/Caching/CacheClientTestsBase.SetAllExpiration.cs
Fixed
Show fixed
Hide fixed
src/Foundatio.TestHarness/Caching/CacheClientTestsBase.SetAllExpiration.cs
Fixed
Show fixed
Hide fixed
src/Foundatio.TestHarness/Caching/CacheClientTestsBase.SetAllExpiration.cs
Fixed
Show fixed
Hide fixed
src/Foundatio.TestHarness/Caching/CacheClientTestsBase.SetExpirationAsync.cs
Fixed
Show fixed
Hide fixed
Renames the test methods to explicitly state that they handle large numbers correctly. This change improves the readability and understanding of the test suite, making it clearer what specific scenarios are being tested.
There are one or two small breaking changes around argument validation where we are now enforcing you cannot pass in a null dictionary, and we are now asserting keys cannot be null or empty (white space is valid). This enforces consistency across our caching implementations (redis) as well as all cache method implementations that already took dictionaries and had null checks.