Repository navigation
Conversation
Replaces a fixed time-based wait with a conditional wait for eviction completion to make tests deterministic. Reduces flakiness and can shorten runtime by proceeding immediately once evictions occur.
- Implemented coverage tests for various size computation modes: Balanced, Fast, Adaptive, and Accurate. - Added tests for null and primitive types, enums, arrays, collections, and complex nested structures. - Included edge cases and cycle detection tests to ensure robustness. - Verified size calculations for anonymous types and custom structs. - Ensured consistent results across repeated calls and different modes.
There was a problem hiding this comment.
Pull Request Overview
This PR improves memory statistics estimations and introduces performance optimizations for BlitzCache v2.1.1. The update focuses on enabling zero-overhead operation when statistics tracking features are disabled and improving the accuracy of memory size calculations.
- Enhanced memory sizing with configurable computation modes and improved object graph traversal
- Zero-overhead conditional statistics when tracking features are disabled
- Deterministic capacity enforcement with better eviction accounting
Reviewed Changes
Copilot reviewed 40 out of 41 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| README.md | Updated documentation with new zero-overhead features and configuration options |
| CHANGELOG.md | Added v2.1.1 release notes detailing performance improvements |
| BlitzCache/Statistics/Speed/* | Reorganized statistics classes into Speed namespace |
| BlitzCache/Statistics/Memory/* | New memory sizing infrastructure with multiple computation modes |
| BlitzCache/Statistics/CacheStatistics.cs | Added conditional size tracking and external removal handling |
| BlitzCache/Capacity/* | New capacity enforcement system with configurable eviction strategies |
| BlitzCache/BlitzCacheOptions.cs | Configuration options class for future-proof setup |
| BlitzCache/Extensions/IServiceCollectionExtensions.cs | Enhanced DI registration with options pattern support |
| BlitzCache/Logging/* | Improved logging to omit disabled tracking sections |
| BlitzCache.Tests/* | Comprehensive test coverage for new features |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| { | ||
| var ft = f.FieldType; | ||
| object? fv = null; | ||
| try { fv = f.GetValue(obj); } catch { } |
There was a problem hiding this comment.
Empty catch blocks suppress all exceptions without logging. Consider logging reflection failures or catching specific exception types (like TargetParameterCountException) for better debugging.
| try { fv = f.GetValue(obj); } catch { } | |
| try { fv = f.GetValue(obj); } | |
| catch (Exception ex) | |
| { | |
| // Log reflection failure for debugging | |
| Console.Error.WriteLine($"Failed to get value for field '{f.Name}' on type '{obj?.GetType().FullName}': {ex}"); | |
| } |
| { | ||
| var ft = f.FieldType; | ||
| object? fv = null; | ||
| try { fv = f.GetValue(structValue); } catch { } |
There was a problem hiding this comment.
Empty catch blocks suppress all exceptions without logging. Consider logging reflection failures or catching specific exception types (like TargetParameterCountException) for better debugging.
| try { fv = f.GetValue(structValue); } catch { } | |
| try { fv = f.GetValue(structValue); } | |
| catch (Exception ex) | |
| { | |
| // Log reflection failure for debugging | |
| Console.Error.WriteLine($"Failed to get value for field '{f.Name}' of type '{type.FullName}': {ex.GetType().Name}: {ex.Message}"); | |
| } |
| long total = _options.ObjectHeaderSize; | ||
| var mode = _options.Mode; | ||
| bool isAdaptive = mode == SizeComputationMode.Adaptive; | ||
| bool isAccurate = mode == SizeComputationMode.Accurate; | ||
| int effectiveMaxDepth = _options.MaxDepth + (isAccurate ? 1 : 0); | ||
| if (isAdaptive) effectiveMaxDepth = _options.MaxDepth; // bounded strictly below Accurate |
There was a problem hiding this comment.
The effectiveMaxDepth calculation logic is complex with mode-specific overrides. Consider extracting this into a separate method like CalculateEffectiveMaxDepth(mode, maxDepth) for better readability and testability.
| internal void OnExternalRemoval(string cacheKey, long sizeBytes, bool countAsEviction) | ||
| { | ||
| // Mark so eviction callback can short-circuit. | ||
| externallyRemoved[cacheKey] = 0; |
There was a problem hiding this comment.
Using 0 as a sentinel value in the ConcurrentDictionary is unclear. Consider using a more descriptive approach like externallyRemoved.TryAdd(cacheKey, true) or a dedicated marker value for better code clarity.
| externallyRemoved[cacheKey] = 0; | |
| externallyRemoved[cacheKey] = true; |
| var heaviestProp = cacheStatsType.GetProperty("HeaviestTrackingEnabled", BindingFlags.NonPublic | BindingFlags.Instance | BindingFlags.Public); | ||
| var slowestProp = cacheStatsType.GetProperty("SlowestTrackingEnabled", BindingFlags.NonPublic | BindingFlags.Instance | BindingFlags.Public); |
There was a problem hiding this comment.
Reflection calls in the logging path on every log operation can impact performance. Consider caching the PropertyInfo objects or using a static detection approach during logger initialization.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Improving memory statistics stimations