quick feedback items

aspnet · Jul 6, 2017 · b411284 · b411284
1 parent 6e93664
commit b411284
Showing 1 changed file with 10 additions and 17 deletions.
diff --git a/src/Microsoft.Extensions.Caching.Memory/MemoryCache.cs b/src/Microsoft.Extensions.Caching.Memory/MemoryCache.cs
@@ -96,12 +96,9 @@ private void SetEntry(CacheEntry entry)
                 return;
             }
 
-            if (_options.SizeLimit != null)
+            if (_options.SizeLimit.HasValue && !entry.Size.HasValue)
             {
-                if (!entry.Size.HasValue)
-                {
-                    throw new InvalidOperationException($"Cache entry must specify a value for {nameof(entry.Size)} when {nameof(_options.SizeLimit)} is set.");
-                }
+                throw new InvalidOperationException($"Cache entry must specify a value for {nameof(entry.Size)} when {nameof(_options.SizeLimit)} is set.");
             }
 
             var utcNow = _options.Clock.UtcNow;
@@ -175,7 +172,7 @@ private void SetEntry(CacheEntry entry)
                 }
                 else
                 {
-                    if (_options.SizeLimit != null)
+                    if (_options.SizeLimit.HasValue)
                     {
                         // Entry could not be added, reset cache size
                         Interlocked.Add(ref _cacheSize, -sizeUpdate);
@@ -191,7 +188,7 @@ private void SetEntry(CacheEntry entry)
             }
             else
             {
-                if (_options.SizeLimit != null && updatedCacheSize > _options.SizeLimit)
+                if (_options.SizeLimit.HasValue && updatedCacheSize > _options.SizeLimit)
                 {
                     // The entry was not added due to overcapacity
                     entry.SetExpired(EvictionReason.Capacity);
@@ -265,7 +262,7 @@ public void Remove(object key)
             CacheEntry entry;
             if (_entries.TryRemove(key, out entry))
             {
-                if (_options.SizeLimit != null)
+                if (_options.SizeLimit.HasValue)
                 {
                     Interlocked.Add(ref _cacheSize, -entry.Size.Value);
                 }
@@ -281,7 +278,7 @@ private void RemoveEntry(CacheEntry entry)
         {
             if (EntriesCollection.Remove(new KeyValuePair<object, CacheEntry>(entry.Key, entry)))
             {
-                if (_options.SizeLimit != null)
+                if (_options.SizeLimit.HasValue)
                 {
                     Interlocked.Add(ref _cacheSize, -entry.Size.Value);
                 }
@@ -323,7 +320,7 @@ private static void ScanForExpiredItems(MemoryCache cache)
 
         private bool UpdateCacheSizeExceedsCapacity(CacheEntry entry, CacheEntry priorEntry, out long sizeUpdate, out long updatedCacheSize)
         {
-            if (_options.SizeLimit == null)
+            if (!_options.SizeLimit.HasValue)
             {
                 sizeUpdate = 0;
                 updatedCacheSize = 0;
@@ -342,9 +339,10 @@ private void TriggerOvercapacityCompaction()
             ThreadPool.QueueUserWorkItem(_ =>
             {
                 var currentSize = Interlocked.Read(ref _cacheSize);
-                if (currentSize > _options.SizeLimit * (1 - _options.RemovalPercentageOnOvercapacityCompaction))
+                var lowWatermark = _options.SizeLimit * (1 - _options.RemovalPercentageOnOvercapacityCompaction);
+                if (currentSize > lowWatermark)
                 {
-                    Compact(currentSize - (long)(_options.SizeLimit * (1 - _options.RemovalPercentageOnOvercapacityCompaction)));
+                    Compact(currentSize - (long)lowWatermark, entry => entry.Size.Value);
                 }
             });
         }
@@ -362,11 +360,6 @@ public void Compact(double percentage)
             Compact(removalCountTarget, _ => 1);
         }
 
-        private void Compact(long removalSizeTarget)
-        {
-            Compact(removalSizeTarget, entry => entry.Size.Value);
-        }
-
         private void Compact(long removalSizeTarget, Func<CacheEntry, long> computeEntrySize)
         {
             var entriesToRemove = new List<CacheEntry>();