From fe723e04c4ef508714d686c4893c435a78c88276 Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Fri, 27 Nov 2020 17:06:15 +0100 Subject: [PATCH 1/5] don't acces expensive CacheEntryHelper.Current if options can't be propagated anyway, +20k RPS for TechEmpower benchmark! --- .../src/CacheEntry.cs | 2 ++ .../src/MemoryCache.cs | 9 ++++++--- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/src/libraries/Microsoft.Extensions.Caching.Memory/src/CacheEntry.cs b/src/libraries/Microsoft.Extensions.Caching.Memory/src/CacheEntry.cs index 71f90e475032af..a3a193fd1e7241 100644 --- a/src/libraries/Microsoft.Extensions.Caching.Memory/src/CacheEntry.cs +++ b/src/libraries/Microsoft.Extensions.Caching.Memory/src/CacheEntry.cs @@ -355,6 +355,8 @@ private static void InvokeCallbacks(CacheEntry entry) } } + internal bool CanPropagateOptions() => _expirationTokens != null || _absoluteExpiration != null; + 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 83de60b556abc8..8075a08dee5a8e 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); From 066b2968f11cc12a9f3b0fb74bde9069b79fe57f Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Fri, 27 Nov 2020 18:20:27 +0100 Subject: [PATCH 2/5] inline the hot paths, +7k RPS --- .../src/CacheEntry.cs | 47 ++++++++++++------- 1 file changed, 31 insertions(+), 16 deletions(-) diff --git a/src/libraries/Microsoft.Extensions.Caching.Memory/src/CacheEntry.cs b/src/libraries/Microsoft.Extensions.Caching.Memory/src/CacheEntry.cs index a3a193fd1e7241..a6a5e020c1b537 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; @@ -220,7 +221,8 @@ public void Dispose() } } - internal bool CheckExpired(DateTimeOffset now) + [MethodImpl(MethodImplOptions.AggressiveInlining)] + internal bool CheckExpired(in DateTimeOffset now) { return _isExpired || CheckForExpiredTime(now) || CheckForExpiredTokens(); } @@ -235,27 +237,40 @@ internal void SetExpired(EvictionReason reason) DetachTokens(); } - private bool CheckForExpiredTime(DateTimeOffset now) + private bool CheckForExpiredTime(in DateTimeOffset now) { - if (_absoluteExpiration.HasValue && _absoluteExpiration.Value <= now) - { - SetExpired(EvictionReason.Expired); - return true; - } + if (_absoluteExpiration == null && _absoluteExpiration == null) + return false; - if (_slidingExpiration.HasValue - && (now - LastAccessed) >= _slidingExpiration) + return SlowPath(now); + + bool SlowPath(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 SlowPath(); + + bool SlowPath() { for (int i = 0; i < _expirationTokens.Count; i++) { @@ -266,8 +281,8 @@ internal bool CheckForExpiredTokens() return true; } } + return false; } - return false; } internal void AttachTokens() From 8bbaad30c6dab3b74bf45b69982135f0a4702194 Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Mon, 30 Nov 2020 14:13:40 +0100 Subject: [PATCH 3/5] prefer .HasValue over null check for nullables, use better names for private helper methods --- .../src/CacheEntry.cs | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/src/libraries/Microsoft.Extensions.Caching.Memory/src/CacheEntry.cs b/src/libraries/Microsoft.Extensions.Caching.Memory/src/CacheEntry.cs index a6a5e020c1b537..b56b69005a18af 100644 --- a/src/libraries/Microsoft.Extensions.Caching.Memory/src/CacheEntry.cs +++ b/src/libraries/Microsoft.Extensions.Caching.Memory/src/CacheEntry.cs @@ -239,12 +239,14 @@ internal void SetExpired(EvictionReason reason) private bool CheckForExpiredTime(in DateTimeOffset now) { - if (_absoluteExpiration == null && _absoluteExpiration == null) + if (!_absoluteExpiration.HasValue && !_absoluteExpiration.HasValue) + { return false; + } - return SlowPath(now); + return FullCheck(now); - bool SlowPath(in DateTimeOffset offset) + bool FullCheck(in DateTimeOffset offset) { if (_absoluteExpiration.HasValue && _absoluteExpiration.Value <= offset) { @@ -266,11 +268,13 @@ bool SlowPath(in DateTimeOffset offset) private bool CheckForExpiredTokens() { if (_expirationTokens == null) + { return false; + } - return SlowPath(); + return CheckTokens(); - bool SlowPath() + bool CheckTokens() { for (int i = 0; i < _expirationTokens.Count; i++) { @@ -370,7 +374,7 @@ private static void InvokeCallbacks(CacheEntry entry) } } - internal bool CanPropagateOptions() => _expirationTokens != null || _absoluteExpiration != null; + internal bool CanPropagateOptions() => _expirationTokens != null || _absoluteExpiration.HasValue; internal void PropagateOptions(CacheEntry parent) { From 1cb3d777fe62c67d90d30866e210680d13402bc2 Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Mon, 30 Nov 2020 14:14:24 +0100 Subject: [PATCH 4/5] if possible, avoid expensive CacheEntryHelper.Current access when adding cache entries --- .../Microsoft.Extensions.Caching.Memory/src/CacheEntry.cs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/libraries/Microsoft.Extensions.Caching.Memory/src/CacheEntry.cs b/src/libraries/Microsoft.Extensions.Caching.Memory/src/CacheEntry.cs index b56b69005a18af..d1001d2d220fb1 100644 --- a/src/libraries/Microsoft.Extensions.Caching.Memory/src/CacheEntry.cs +++ b/src/libraries/Microsoft.Extensions.Caching.Memory/src/CacheEntry.cs @@ -216,7 +216,11 @@ public void Dispose() if (_valueHasBeenSet) { _notifyCacheEntryCommit(this); - PropagateOptions(CacheEntryHelper.Current); + + if (CanPropagateOptions()) + { + PropagateOptions(CacheEntryHelper.Current); + } } } } From 8381ed66f2cd03436a994ed4e6e80415f7ec786f Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Mon, 30 Nov 2020 17:10:19 +0100 Subject: [PATCH 5/5] Update src/libraries/Microsoft.Extensions.Caching.Memory/src/CacheEntry.cs --- .../Microsoft.Extensions.Caching.Memory/src/CacheEntry.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libraries/Microsoft.Extensions.Caching.Memory/src/CacheEntry.cs b/src/libraries/Microsoft.Extensions.Caching.Memory/src/CacheEntry.cs index d1001d2d220fb1..7b2a098d950573 100644 --- a/src/libraries/Microsoft.Extensions.Caching.Memory/src/CacheEntry.cs +++ b/src/libraries/Microsoft.Extensions.Caching.Memory/src/CacheEntry.cs @@ -243,7 +243,7 @@ internal void SetExpired(EvictionReason reason) private bool CheckForExpiredTime(in DateTimeOffset now) { - if (!_absoluteExpiration.HasValue && !_absoluteExpiration.HasValue) + if (!_absoluteExpiration.HasValue && !_slidingExpiration.HasValue) { return false; }