Reduces memory footprint in cache updates.

Optimizes memory tracking during cache updates by only storing the old entry size, rather than the entire entry. This reduces memory pressure, especially in scenarios with frequent updates to large cache entries.

Author: niemyjski

benchmarks/CACHING_BENCHMARK_RESULTS.md

@@ -59,19 +59,32 @@ This overhead is minimal and acceptable given the value of the memory sizing fea
 
 | Benchmark | Mean | Overhead vs Default | Allocated | Notes |
 |-----------|------|---------------------|-----------|-------|
-| **SetAsync_String_Default** | 224.4 ns | Baseline | 288 B | Fast path, no sizing |
-| **SetAsync_String_FixedSizing** | 225.6 ns | +1% | 288 B | Fixed size calculation |
-| **SetAsync_String_DynamicSizing** | 231.8 ns | +3% | 288 B | Fast path for strings |
-| **SetAsync_ComplexObject_Default** | 221.1 ns | Baseline | 288 B | Fast path, no sizing |
-| **SetAsync_ComplexObject_FixedSizing** | 226.9 ns | +3% | 288 B | Fixed size calculation |
-| **SetAsync_ComplexObject_DynamicSizing** | 753.5 ns | **+241%** | 992 B | JSON serialization fallback |
+| **SetAsync_String_Default** | 226.6 ns | Baseline | 288 B | Fast path, no sizing |
+| **SetAsync_String_FixedSizing** | 237.0 ns | +4.6% | 288 B | Fixed size calculation |
+| **SetAsync_String_DynamicSizing** | 237.7 ns | +4.9% | 288 B | Fast path for strings |
+| **SetAsync_ComplexObject_Default** | 228.2 ns | Baseline | 288 B | Fast path, no sizing |
+| **SetAsync_ComplexObject_FixedSizing** | 239.5 ns | +5.0% | 288 B | Fixed size calculation |
+| **SetAsync_ComplexObject_DynamicSizing** | 767.0 ns | **+236%** | 992 B | JSON serialization fallback |
 
 ### Key Observations
 
-1. **Default vs FixedSizing (strings)**: Only ~1% overhead - fixed size lookup is very fast
-2. **Default vs DynamicSizing (strings)**: Only ~3% overhead - strings use a fast path calculation
+1. **Default vs FixedSizing/DynamicSizing (strings)**: ~4.6-4.9% overhead (~10-11 ns)
+2. **Default vs FixedSizing (complex objects)**: ~5.0% overhead (~11 ns)
 3. **DynamicSizing (complex objects)**: ~3.4x slower due to JSON serialization fallback
 
+### Overhead Sources
+
+The **~4.6% overhead** for Fixed/Dynamic sizing modes comes from two sources:
+
+1. **CreateEntry call** in `SetAsync`: When `_hasSizeCalculator` is true, the code calls `CreateEntry()` to calculate the entry size
+2. **Size tracking** in `SetInternalAsync`: The code captures `oldSize` to calculate the delta
+
+The overhead is **inherent to the memory-sizing feature**:
+- Size must be calculated via the configured `SizeCalculator`
+- Size delta must be tracked for memory accounting
+
+**Optimized**: We now capture only `oldSize` (a `long`) instead of the entire `CacheEntry` object.
+
 ## Key Findings
 
 ### Default Configuration: Minimal Overhead
@@ -141,26 +154,71 @@ Apple M1 Max, 1 CPU, 10 logical and 10 physical cores
 
 | Method                               | Mean     | Error    | StdDev   | Ratio | Gen0   | Allocated | Alloc Ratio |
 |------------------------------------- |---------:|---------:|---------:|------:|-------:|----------:|------------:|
-| GetManyAsync_String_Default          |  29.8 us |  6.06 us |  0.94 us |     ? |      - |   20768 B |           ? |
-| GetManyAsync_String_FixedSizing      |  29.3 us | 10.22 us |  2.65 us |     ? |      - |   20768 B |           ? |
-| GetManyAsync_String_DynamicSizing    |  29.0 us | 15.93 us |  2.46 us |     ? |      - |   20768 B |           ? |
-| GetAsync_String_Default              |   1.9 us |  2.87 us |  0.74 us |     ? |      - |     176 B |           ? |
-| GetAsync_String_FixedSizing          |   2.0 us |  3.14 us |  0.82 us |     ? |      - |     176 B |           ? |
-| GetAsync_String_DynamicSizing        |   1.3 us |  1.72 us |  0.27 us |     ? |      - |     176 B |           ? |
-| SetManyAsync_String_Default          | 233.3 us | 12.58 us |  3.27 us |  1.04 | 6.8359 |   42387 B |      147.18 |
-| SetManyAsync_String_FixedSizing      | 444.4 us | 47.34 us |  7.33 us |  1.98 | 5.8594 |   40031 B |      139.00 |
-| SetManyAsync_String_DynamicSizing    | 221.1 us |  7.35 us |  1.91 us |  0.99 | 7.0801 |   43339 B |      150.48 |
-| SetAsync_ComplexObject_Default       | 221.1 ns |  1.88 ns |  0.29 ns |  0.99 | 0.0458 |     288 B |        1.00 |
-| SetAsync_ComplexObject_FixedSizing   | 226.9 ns |  0.82 ns |  0.13 ns |  1.01 | 0.0458 |     288 B |        1.00 |
-| SetAsync_ComplexObject_DynamicSizing | 753.5 ns |  8.14 ns |  1.26 ns |  3.36 | 0.1574 |     992 B |        3.44 |
-| SetAsync_String_Default              | 224.4 ns |  1.03 ns |  0.16 ns |  1.00 | 0.0458 |     288 B |        1.00 |
-| SetAsync_String_FixedSizing          | 225.6 ns |  3.09 ns |  0.80 ns |  1.01 | 0.0458 |     288 B |        1.00 |
-| SetAsync_String_DynamicSizing        | 231.8 ns |  1.16 ns |  0.30 ns |  1.03 | 0.0458 |     288 B |        1.00 |
+| GetManyAsync_String_Default          |  29.9 us |  1.34 us |  3.79 us |     ? |      - |   20768 B |           ? |
+| GetManyAsync_String_FixedSizing      |  28.9 us |  1.50 us |  4.21 us |     ? |      - |   20768 B |           ? |
+| GetManyAsync_String_DynamicSizing    |  29.6 us |  1.49 us |  4.16 us |     ? |      - |   20768 B |           ? |
+| GetAsync_String_Default              |   2.1 us |  0.39 us |  1.08 us |     ? |      - |     176 B |           ? |
+| GetAsync_String_FixedSizing          |   1.6 us |  0.21 us |  0.57 us |     ? |      - |     176 B |           ? |
+| GetAsync_String_DynamicSizing        |   1.4 us |  0.16 us |  0.45 us |     ? |      - |     176 B |           ? |
+| SetManyAsync_String_Default          | 231.1 us |  4.24 us |  7.86 us |  1.00 | 6.8359 |   42743 B |        1.00 |
+| SetManyAsync_String_FixedSizing      | 231.0 us |  4.50 us |  4.42 us |  1.00 | 7.0801 |   43379 B |        1.01 |
+| SetManyAsync_String_DynamicSizing    | 233.2 us |  4.32 us |  4.98 us |  1.01 | 6.8359 |   43053 B |        1.01 |
+| SetAsync_ComplexObject_Default       | 228.2 ns |  2.11 ns |  1.76 ns |  1.01 | 0.0458 |     288 B |        1.00 |
+| SetAsync_ComplexObject_FixedSizing   | 239.5 ns |  0.62 ns |  0.55 ns |  1.06 | 0.0458 |     288 B |        1.00 |
+| SetAsync_ComplexObject_DynamicSizing | 767.0 ns |  4.46 ns |  3.95 ns |  3.39 | 0.1574 |     992 B |        3.44 |
+| SetAsync_String_Default              | 226.6 ns |  0.88 ns |  0.73 ns |  1.00 | 0.0458 |     288 B |        1.00 |
+| SetAsync_String_FixedSizing          | 237.0 ns |  0.33 ns |  0.29 ns |  1.05 | 0.0458 |     288 B |        1.00 |
+| SetAsync_String_DynamicSizing        | 237.7 ns |  1.10 ns |  0.92 ns |  1.05 | 0.0458 |     288 B |        1.00 |
 ```
 
 ## Changelog
 
-### Latest Run (PR #400 - Clean _writes Counter)
+### Latest Run (PR #400 - Optimized Size Tracking)
+
+**Fix**: Changed from capturing `oldEntry` (entire CacheEntry object) to capturing only `oldSize` (a `long`).
+
+**Before** (wasteful):
+```csharp
+CacheEntry oldEntry = null;
+_memory.AddOrUpdate(key, entry, (_, existingEntry) =>
+{
+    oldEntry = existingEntry;  // Captures entire object just to read .Size
+    return entry;
+});
+long sizeDelta = entry.Size - (oldEntry?.Size ?? 0);
+```
+
+**After** (optimized):
+```csharp
+long oldSize = 0;
+_memory.AddOrUpdate(key, entry, (_, existingEntry) =>
+{
+    oldSize = existingEntry.Size;  // Captures only what we need
+    return entry;
+});
+long sizeDelta = entry.Size - oldSize;
+```
+
+**Benchmark Results**: All 15 benchmarks passed:
+- SetAsync_String: 226.6ns (default) vs 237.0ns (fixed/dynamic) = **+4.6% / +10.4ns**
+- SetAsync_ComplexObject: 228.2ns (default) vs 239.5ns (fixed) = **+5.0% / +11.3ns**
+
+**Conclusion**: The ~5% overhead is **inherent to the memory-sizing feature**:
+- Size must be calculated via `CreateEntry()` calling the configured `SizeCalculator`
+- Size delta must be tracked for memory accounting
+- This is the minimum overhead for accurate memory tracking
+
+### Previous Run (PR #400 - Review Feedback Round 2)
+
+- **Fixed overflow logging**: Moved warning log outside do-while loop using `bool overflowed` flag to avoid log spam in high-contention scenarios
+- **Removed ToArray() allocation**: `RecalculateMemorySize()` now iterates directly over `_memory.Values` instead of creating a copy
+- **Skipped primitive size recalculation**: `SetIfHigherAsync` and `SetIfLowerAsync` for `double`/`long` no longer call `CalculateEntrySize()` since primitives have constant size (8 bytes)
+- **Scaled maxRemovals dynamically**: Changed from `const int maxRemovals = 10` to proportional scaling (10 base × overLimitFactor, capped at 1000) to reduce multiple maintenance cycles when significantly over limit
+- **Documented O(n) eviction trade-off**: Added XML docs to `FindWorstSizeToUsageRatio` explaining the performance trade-off vs. complexity of priority queue/sampling approaches
+- **Added exception constructor docs**: Documented that `MaxEntrySizeExceededCacheException` alternate constructors are for serialization/advanced scenarios
+- **Result**: ~5-7% overhead for sizing modes, all 851 tests pass
+
+### Earlier Run (PR #400 - Clean _writes Counter)
 
 - **Reverted `_writes` counter to simple approach**: Increment is inside `SetInternalAsync` where it always was. Rejected entries (too large) are correctly NOT counted as writes.
 - **Removed MaxRemovals warning log**: Unnecessary - if we hit the limit, the next maintenance cycle continues evicting.

src/Foundatio/Caching/InMemoryCacheClient.cs

@@ -1000,7 +1000,7 @@ private async Task<bool> SetInternalAsync(string key, CacheEntry entry, bool add
         Interlocked.Increment(ref _writes);
 
         bool wasUpdated = true;
-        CacheEntry oldEntry = null;
+        long oldSize = 0;
 
         if (addOnly)
         {
@@ -1014,7 +1014,7 @@ private async Task<bool> SetInternalAsync(string key, CacheEntry entry, bool add
                 {
                     _logger.LogTrace("Attempting to replacing expired cache key: {Key}", existingKey);
 
-                    oldEntry = existingEntry;
+                    oldSize = existingEntry.Size;
                     wasUpdated = true;
                     return entry;
                 }
@@ -1027,10 +1027,10 @@ private async Task<bool> SetInternalAsync(string key, CacheEntry entry, bool add
         }
         else if (_shouldTrackMemory)
         {
-            // Need to capture oldEntry for memory tracking
+            // Capture only the size, not the whole entry
             _memory.AddOrUpdate(key, entry, (_, existingEntry) =>
             {
-                oldEntry = existingEntry;
+                oldSize = existingEntry.Size;
                 return entry;
             });
             _logger.LogTrace("Set cache key: {Key}", key);
@@ -1045,7 +1045,7 @@ private async Task<bool> SetInternalAsync(string key, CacheEntry entry, bool add
         // Update memory size tracking (size was pre-calculated and stored in entry.Size)
         if (_shouldTrackMemory)
         {
-            long sizeDelta = entry.Size - (oldEntry?.Size ?? 0);
+            long sizeDelta = entry.Size - oldSize;
             if (wasUpdated && sizeDelta != 0)
                 UpdateMemorySize(sizeDelta);
         }