HybridCache : implement the tag expiration feature

eng/MSBuild/LegacySupport.props

@@ -15,7 +15,7 @@
     <Compile Include="$(MSBuildThisFileDirectory)\..\..\src\LegacySupport\CallerAttributes\*.cs" LinkBase="LegacySupport\CallerAttributes" />
   </ItemGroup>
 
-  <ItemGroup Condition="'$(InjectSkipLocalsInitAttributeOnLegacy)' == 'true' AND ('$(TargetFramework)' == 'net462' or '$(TargetFramework)' == 'netstandard2.0' or '$(TargetFramework)' == 'netcoreapp3.1')">
+  <ItemGroup Condition="'$(InjectSkipLocalsInitAttributeOnLegacy)' == 'true' AND ('$(TargetFramework)' == 'net462' or '$(TargetFramework)' == 'netstandard2.0' or '$(TargetFramework)' == 'netstandard2.1' or '$(TargetFramework)' == 'netcoreapp3.1')">
     <Compile Include="$(MSBuildThisFileDirectory)\..\..\src\LegacySupport\SkipLocalsInitAttribute\*.cs" LinkBase="LegacySupport\SkipLocalsInitAttribute" />
   </ItemGroup>
 

eng/packages/TestOnly.props

@@ -16,6 +16,7 @@
     <PackageVersion Include="Verify.Xunit" Version="20.4.0" />
     <PackageVersion Include="Xunit.Combinatorial" Version="1.6.24" />
     <PackageVersion Include="xunit.extensibility.execution" Version="2.4.2" />
+    <PackageVersion Include="Xunit.SkippableFact" Version="1.4.13" />
   </ItemGroup>
 
   <ItemGroup Condition="'$(TargetFramework)' == 'net462'">

src/Libraries/Microsoft.Extensions.Caching.Hybrid/HybridCacheOptions.cs

@@ -11,11 +11,13 @@ public class HybridCacheOptions
     private const int ShiftBytesToMibiBytes = 20;
 
     /// <summary>
-    /// Gets or sets the default global options to be applied to <see cref="HybridCache"/> operations; if options are
-    /// specified at the individual call level, the non-null values are merged (with the per-call
-    /// options being used in preference to the global options). If no value is specified for a given
-    /// option (globally or per-call), the implementation may choose a reasonable default.
+    /// Gets or sets the default global options to be applied to <see cref="HybridCache"/> operations.
     /// </summary>
+    /// <remarks>
+    /// If options are specified at the individual call level, the non-null values are merged
+    /// (with the per-call options being used in preference to the global options). If no value is
+    /// specified for a given option (globally or per-call), the implementation can choose a reasonable default.
+    /// </remarks>
     public HybridCacheEntryOptions? DefaultEntryOptions { get; set; }
 
     /// <summary>
@@ -24,21 +26,35 @@ public class HybridCacheOptions
     public bool DisableCompression { get; set; }
 
     /// <summary>
-    /// Gets or sets the maximum size of cache items; attempts to store values over this size will be logged
-    /// and the value will not be stored in cache.
+    /// Gets or sets the maximum size of cache items.
     /// </summary>
-    /// <remarks>The default value is 1 MiB.</remarks>
+    /// <value>
+    /// The maximum size of cache items. The default value is 1 MiB.
+    /// </value>
+    /// <remarks>
+    /// Attempts to store values over this size are logged,
+    /// and the value isn't stored in the cache.
+    /// </remarks>
     public long MaximumPayloadBytes { get; set; } = 1 << ShiftBytesToMibiBytes; // 1MiB
 
     /// <summary>
-    /// Gets or sets the maximum permitted length (in characters) of keys; attempts to use keys over this size will be logged.
+    /// Gets or sets the maximum permitted length (in characters) of keys.
     /// </summary>
-    /// <remark>The default value is 1024 characters.</remark>
+    /// <value>
+    /// The maximum permitted length of keys, in characters. The default value is 1024 characters.
+    /// </value>
+    /// <remarks>Attempts to use keys over this size are logged.</remarks>
     public int MaximumKeyLength { get; set; } = 1024; // characters
 
     /// <summary>
-    /// Gets or sets a value indicating whether to use "tags" data as dimensions on metric reporting; if enabled, care should be used to ensure that
-    /// tags do not contain data that should not be visible in metrics systems.
+    /// Gets or sets a value indicating whether to use "tags" data as dimensions on metric reporting.
     /// </summary>
+    /// <value>
+    /// <see langword="true"/> to use "tags" data as dimensions on metric reporting; otherwise, <see langword="false"/>.
+    /// </value>
+    /// <remarks>
+    /// If enabled, take care to ensure that tags don't contain data that
+    /// should not be visible in metrics systems.
+    /// </remarks>
     public bool ReportTagMetrics { get; set; }
 }

src/Libraries/Microsoft.Extensions.Caching.Hybrid/Internal/BufferChunk.cs

@@ -15,20 +15,23 @@ namespace Microsoft.Extensions.Caching.Hybrid.Internal;
 internal readonly struct BufferChunk
 {
     private const int FlagReturnToPool = (1 << 31);
-
     private readonly int _lengthAndPoolFlag;
 
-    public byte[]? Array { get; } // null for default
+    public byte[]? OversizedArray { get; } // null for default
+
+    public bool HasValue => OversizedArray is not null;
 
+    public int Offset { get; }
     public int Length => _lengthAndPoolFlag & ~FlagReturnToPool;
 
     public bool ReturnToPool => (_lengthAndPoolFlag & FlagReturnToPool) != 0;
 
     public BufferChunk(byte[] array)
     {
         Debug.Assert(array is not null, "expected valid array input");
-        Array = array;
+        OversizedArray = array;
         _lengthAndPoolFlag = array!.Length;
+        Offset = 0;
 
         // assume not pooled, if exact-sized
         // (we don't expect array.Length to be negative; we're really just saying
@@ -39,11 +42,12 @@ public BufferChunk(byte[] array)
         Debug.Assert(Length == array.Length, "array length not respected");
     }
 
-    public BufferChunk(byte[] array, int length, bool returnToPool)
+    public BufferChunk(byte[] array, int offset, int length, bool returnToPool)
     {
         Debug.Assert(array is not null, "expected valid array input");
         Debug.Assert(length >= 0, "expected valid length");
-        Array = array;
+        OversizedArray = array;
+        Offset = offset;
         _lengthAndPoolFlag = length | (returnToPool ? FlagReturnToPool : 0);
         Debug.Assert(ReturnToPool == returnToPool, "return-to-pool not respected");
         Debug.Assert(Length == length, "length not respected");
@@ -58,7 +62,7 @@ public byte[] ToArray()
         }
 
         var copy = new byte[length];
-        Buffer.BlockCopy(Array!, 0, copy, 0, length);
+        Buffer.BlockCopy(OversizedArray!, Offset, copy, 0, length);
         return copy;
 
         // Note on nullability of Array; the usage here is that a non-null array
@@ -73,15 +77,19 @@ internal void RecycleIfAppropriate()
     {
         if (ReturnToPool)
         {
-            ArrayPool<byte>.Shared.Return(Array!);
+            ArrayPool<byte>.Shared.Return(OversizedArray!);
         }
 
         Unsafe.AsRef(in this) = default; // anti foot-shotgun double-return guard; not 100%, but worth doing
-        Debug.Assert(Array is null && !ReturnToPool, "expected clean slate after recycle");
+        Debug.Assert(OversizedArray is null && !ReturnToPool, "expected clean slate after recycle");
     }
 
+    internal ArraySegment<byte> AsArraySegment() => Length == 0 ? default! : new(OversizedArray!, Offset, Length);
+
+    internal ReadOnlySpan<byte> AsSpan() => Length == 0 ? default : new(OversizedArray!, Offset, Length);
+
     // get the data as a ROS; for note on null-logic of Array!, see comment in ToArray
-    internal ReadOnlySequence<byte> AsSequence() => Length == 0 ? default : new ReadOnlySequence<byte>(Array!, 0, Length);
+    internal ReadOnlySequence<byte> AsSequence() => Length == 0 ? default : new ReadOnlySequence<byte>(OversizedArray!, Offset, Length);
 
     internal BufferChunk DoNotReturnToPool()
     {

src/Libraries/Microsoft.Extensions.Caching.Hybrid/Internal/DefaultHybridCache.CacheItem.cs

@@ -3,29 +3,46 @@
 
 using System;
 using System.Diagnostics;
+using System.Runtime.CompilerServices;
 using System.Threading;
 using Microsoft.Extensions.Caching.Memory;
+using Microsoft.Extensions.Logging;
 
 namespace Microsoft.Extensions.Caching.Hybrid.Internal;
 
 internal partial class DefaultHybridCache
 {
     internal abstract class CacheItem
     {
+        private readonly long _creationTimestamp;
+
+        protected CacheItem(long creationTimestamp, TagSet tags)
+        {
+            Tags = tags;
+            _creationTimestamp = creationTimestamp;
+        }
+
         private int _refCount = 1; // the number of pending operations against this cache item
 
         public abstract bool DebugIsImmutable { get; }
 
+        public long CreationTimestamp => _creationTimestamp;
+
+        public TagSet Tags { get; }
+
         // Note: the ref count is the number of callers anticipating this value at any given time. Initially,
         // it is one for a simple "get the value" flow, but if another call joins with us, it'll be incremented.
         // If either cancels, it will get decremented, with the entire flow being cancelled if it ever becomes
         // zero.
         // This counter also drives cache lifetime, with the cache itself incrementing the count by one. In the
         // case of mutable data, cache eviction may reduce this to zero (in cooperation with any concurrent readers,
-        // who incr/decr around their fetch), allowing safe buffer recycling.
+        // who increment/decrement around their fetch), allowing safe buffer recycling.
 
         internal int RefCount => Volatile.Read(ref _refCount);
 
+        internal void UnsafeSetCreationTimestamp(long timestamp)
+            => Unsafe.AsRef(in _creationTimestamp) = timestamp;
+
         internal static readonly PostEvictionDelegate SharedOnEviction = static (key, value, reason, state) =>
         {
             if (value is CacheItem item)
@@ -87,15 +104,25 @@ protected virtual void OnFinalRelease() // any required release semantics
 
     internal abstract class CacheItem<T> : CacheItem
     {
+        protected CacheItem(long creationTimestamp, TagSet tags)
+            : base(creationTimestamp, tags)
+        {
+        }
+
         public abstract bool TryGetSize(out long size);
 
-        // attempt to get a value that was *not* previously reserved
-        public abstract bool TryGetValue(out T value);
+        // Attempt to get a value that was *not* previously reserved.
+        // Note on ILogger usage: we don't want to propagate and store this everywhere.
+        // It is used for reporting deserialization problems - pass it as needed.
+        // (CacheItem gets into the IMemoryCache - let's minimize the onward reachable set
+        // of that cache, by only handing it leaf nodes of a "tree", not a "graph" with
+        // backwards access - we can also limit object size at the same time)
+        public abstract bool TryGetValue(ILogger log, out T value);
 
         // get a value that *was* reserved, countermanding our reservation in the process
-        public T GetReservedValue()
+        public T GetReservedValue(ILogger log)
         {
-            if (!TryGetValue(out var value))
+            if (!TryGetValue(log, out var value))
             {
                 Throw();
             }
@@ -106,6 +133,7 @@ public T GetReservedValue()
             static void Throw() => throw new ObjectDisposedException("The cache item has been recycled before the value was obtained");
         }
 
-        internal static CacheItem<T> Create() => ImmutableTypeCache<T>.IsImmutable ? new ImmutableCacheItem<T>() : new MutableCacheItem<T>();
+        internal static CacheItem<T> Create(long creationTimestamp, TagSet tags) => ImmutableTypeCache<T>.IsImmutable
+            ? new ImmutableCacheItem<T>(creationTimestamp, tags) : new MutableCacheItem<T>(creationTimestamp, tags);
     }
 }

src/Libraries/Microsoft.Extensions.Caching.Hybrid/Internal/DefaultHybridCache.Debug.cs

@@ -54,7 +54,6 @@ private partial class MutableCacheItem<T>
 #endif
 
         [Conditional("DEBUG")]
-        [SuppressMessage("Performance", "CA1822:Mark members as static", Justification = "Instance state used in debug")]
         internal void DebugOnlyTrackBuffer(DefaultHybridCache cache)
         {
 #if DEBUG
@@ -63,18 +62,21 @@ internal void DebugOnlyTrackBuffer(DefaultHybridCache cache)
             {
                 _cache?.DebugOnlyIncrementOutstandingBuffers();
             }
+#else
+            _ = this; // dummy just to prevent CA1822, never hit
 #endif
         }
 
         [Conditional("DEBUG")]
-        [SuppressMessage("Performance", "CA1822:Mark members as static", Justification = "Instance state used in debug")]
         private void DebugOnlyDecrementOutstandingBuffers()
         {
 #if DEBUG
             if (_buffer.ReturnToPool)
             {
                 _cache?.DebugOnlyDecrementOutstandingBuffers();
             }
+#else
+            _ = this; // dummy just to prevent CA1822, never hit
 #endif
         }
     }

src/Libraries/Microsoft.Extensions.Caching.Hybrid/Internal/DefaultHybridCache.ImmutableCacheItem.cs

@@ -2,6 +2,7 @@
 // The .NET Foundation licenses this file to you under the MIT license.
 
 using System.Threading;
+using Microsoft.Extensions.Logging;
 
 namespace Microsoft.Extensions.Caching.Hybrid.Internal;
 
@@ -11,6 +12,11 @@ internal partial class DefaultHybridCache
     {
         private static ImmutableCacheItem<T>? _sharedDefault;
 
+        public ImmutableCacheItem(long creationTimestamp, TagSet tags)
+            : base(creationTimestamp, tags)
+        {
+        }
+
         private T _value = default!; // deferred until SetValue
 
         public long Size { get; private set; } = -1;
@@ -24,7 +30,7 @@ public static ImmutableCacheItem<T> GetReservedShared()
             ImmutableCacheItem<T>? obj = Volatile.Read(ref _sharedDefault);
             if (obj is null || !obj.TryReserve())
             {
-                obj = new();
+                obj = new(0, TagSet.Empty); // timestamp doesn't matter - not used in L1/L2
                 _ = obj.TryReserve(); // this is reliable on a new instance
                 Volatile.Write(ref _sharedDefault, obj);
             }
@@ -38,7 +44,7 @@ public void SetValue(T value, long size)
             Size = size;
         }
 
-        public override bool TryGetValue(out T value)
+        public override bool TryGetValue(ILogger log, out T value)
         {
             value = _value;
             return true; // always available