From a2f81e7258d2963af52d4441e891025aa46a1fc3 Mon Sep 17 00:00:00 2001
From: Adam Sitnik <adam.sitnik@gmail.com>
Date: Tue, 1 Dec 2020 06:56:23 +0100
Subject: [PATCH] More MemoryCache perf improvements (#45280)

* don't acces expensive CacheEntryHelper.Current if options can't be propagated anyway, +20k RPS for TechEmpower benchmark!

* inline the hot paths, +7k RPS

* prefer .HasValue over null check for nullables, use better names for private helper methods

* if possible, avoid expensive CacheEntryHelper.Current access when adding cache entries
---
 .../src/CacheEntry.cs                         | 55 ++++++++++++++-----
 .../src/MemoryCache.cs                        |  9 ++-
 2 files changed, 46 insertions(+), 18 deletions(-)

diff --git a/src/libraries/Microsoft.Extensions.Caching.Memory/src/CacheEntry.cs b/src/libraries/Microsoft.Extensions.Caching.Memory/src/CacheEntry.cs
index 71f90e475032a..7b2a098d95057 100644
--- a/src/libraries/Microsoft.Extensions.Caching.Memory/src/CacheEntry.cs
+++ b/src/libraries/Microsoft.Extensions.Caching.Memory/src/CacheEntry.cs
@@ -3,6 +3,7 @@
 
 using System;
 using System.Collections.Generic;
+using System.Runtime.CompilerServices;
 using System.Threading;
 using System.Threading.Tasks;
 using Microsoft.Extensions.Logging;
@@ -215,12 +216,17 @@ public void Dispose()
                 if (_valueHasBeenSet)
                 {
                     _notifyCacheEntryCommit(this);
-                    PropagateOptions(CacheEntryHelper.Current);
+
+                    if (CanPropagateOptions())
+                    {
+                        PropagateOptions(CacheEntryHelper.Current);
+                    }
                 }
             }
         }
 
-        internal bool CheckExpired(DateTimeOffset now)
+        [MethodImpl(MethodImplOptions.AggressiveInlining)]
+        internal bool CheckExpired(in DateTimeOffset now)
         {
             return _isExpired || CheckForExpiredTime(now) || CheckForExpiredTokens();
         }
@@ -235,27 +241,44 @@ internal void SetExpired(EvictionReason reason)
             DetachTokens();
         }
 
-        private bool CheckForExpiredTime(DateTimeOffset now)
+        private bool CheckForExpiredTime(in DateTimeOffset now)
         {
-            if (_absoluteExpiration.HasValue && _absoluteExpiration.Value <= now)
+            if (!_absoluteExpiration.HasValue && !_slidingExpiration.HasValue)
             {
-                SetExpired(EvictionReason.Expired);
-                return true;
+                return false;
             }
 
-            if (_slidingExpiration.HasValue
-                && (now - LastAccessed) >= _slidingExpiration)
+            return FullCheck(now);
+
+            bool FullCheck(in DateTimeOffset offset)
             {
-                SetExpired(EvictionReason.Expired);
-                return true;
-            }
+                if (_absoluteExpiration.HasValue && _absoluteExpiration.Value <= offset)
+                {
+                    SetExpired(EvictionReason.Expired);
+                    return true;
+                }
 
-            return false;
+                if (_slidingExpiration.HasValue
+                    && (offset - LastAccessed) >= _slidingExpiration)
+                {
+                    SetExpired(EvictionReason.Expired);
+                    return true;
+                }
+
+                return false;
+            }
         }
 
-        internal bool CheckForExpiredTokens()
+        private bool CheckForExpiredTokens()
         {
-            if (_expirationTokens != null)
+            if (_expirationTokens == null)
+            {
+                return false;
+            }
+
+            return CheckTokens();
+
+            bool CheckTokens()
             {
                 for (int i = 0; i < _expirationTokens.Count; i++)
                 {
@@ -266,8 +289,8 @@ internal bool CheckForExpiredTokens()
                         return true;
                     }
                 }
+                return false;
             }
-            return false;
         }
 
         internal void AttachTokens()
@@ -355,6 +378,8 @@ private static void InvokeCallbacks(CacheEntry entry)
             }
         }
 
+        internal bool CanPropagateOptions() => _expirationTokens != null || _absoluteExpiration.HasValue;
+
         internal void PropagateOptions(CacheEntry parent)
         {
             if (parent == null)
diff --git a/src/libraries/Microsoft.Extensions.Caching.Memory/src/MemoryCache.cs b/src/libraries/Microsoft.Extensions.Caching.Memory/src/MemoryCache.cs
index 83de60b556abc..8075a08dee5a8 100644
--- a/src/libraries/Microsoft.Extensions.Caching.Memory/src/MemoryCache.cs
+++ b/src/libraries/Microsoft.Extensions.Caching.Memory/src/MemoryCache.cs
@@ -250,9 +250,12 @@ public bool TryGetValue(object key, out object result)
                     entry.LastAccessed = utcNow;
                     result = entry.Value;
 
-                    // When this entry is retrieved in the scope of creating another entry,
-                    // that entry needs a copy of these expiration tokens.
-                    entry.PropagateOptions(CacheEntryHelper.Current);
+                    if (entry.CanPropagateOptions())
+                    {
+                        // When this entry is retrieved in the scope of creating another entry,
+                        // that entry needs a copy of these expiration tokens.
+                        entry.PropagateOptions(CacheEntryHelper.Current);
+                    }
 
                     StartScanForExpiredItemsIfNeeded(utcNow);