Skip to content

Commit

Permalink
Reduce CacheEntry size (#45410)
Browse files Browse the repository at this point in the history 
* replace 3 boolean fields with one enum flag field

* use auto property for AbsoluteExpiration

* simplify the code by using expression bodies

* don't use internal fields of CacheEntry, the public properties are enough

* expose private methods of MemoryCache and replace the usage of delegates with dependency to MemoryCache

2 fields less for MemoryCache and three for every CacheEntry
  • Loading branch information
@adamsitnik
adamsitnik authored Dec 2, 2020
1 parent 63b9b20 commit 62b4a1f
Show file tree
Hide file tree
Showing 3 changed files with 94 additions and 153 deletions.
165 changes: 62 additions & 103 deletions src/libraries/Microsoft.Extensions.Caching.Memory/src/CacheEntry.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,86 +13,44 @@ namespace Microsoft.Extensions.Caching.Memory
{
internal class CacheEntry : ICacheEntry
{
private bool _disposed;
private static readonly Action<object> ExpirationCallback = ExpirationTokensExpired;
private readonly Action<CacheEntry> _notifyCacheOfExpiration;
private readonly Action<CacheEntry> _notifyCacheEntryCommit;

private readonly object _lock = new object();
private readonly MemoryCache _cache;

private IList<IDisposable> _expirationTokenRegistrations;
private IList<PostEvictionCallbackRegistration> _postEvictionCallbacks;
private bool _isExpired;
private readonly ILogger _logger;

internal IList<IChangeToken> _expirationTokens;
internal DateTimeOffset? _absoluteExpiration;
internal TimeSpan? _absoluteExpirationRelativeToNow;
private IList<IChangeToken> _expirationTokens;
private TimeSpan? _absoluteExpirationRelativeToNow;
private TimeSpan? _slidingExpiration;
private long? _size;
private IDisposable _scope;
private object _value;
private bool _valueHasBeenSet;

internal readonly object _lock = new object();
private int _state; // actually a [Flag] enum called "State"

internal CacheEntry(
object key,
Action<CacheEntry> notifyCacheEntryCommit,
Action<CacheEntry> notifyCacheOfExpiration,
ILogger logger)
internal CacheEntry(object key, MemoryCache memoryCache)
{
if (key == null)
{
throw new ArgumentNullException(nameof(key));
}

if (notifyCacheEntryCommit == null)
{
throw new ArgumentNullException(nameof(notifyCacheEntryCommit));
}

if (notifyCacheOfExpiration == null)
{
throw new ArgumentNullException(nameof(notifyCacheOfExpiration));
}

if (logger == null)
{
throw new ArgumentNullException(nameof(logger));
}

Key = key;
_notifyCacheEntryCommit = notifyCacheEntryCommit;
_notifyCacheOfExpiration = notifyCacheOfExpiration;

Key = key ?? throw new ArgumentNullException(nameof(key));
_cache = memoryCache ?? throw new ArgumentNullException(nameof(memoryCache));
_scope = CacheEntryHelper.EnterScope(this);
_logger = logger;
}

/// <summary>
/// Gets or sets an absolute expiration date for the cache entry.
/// </summary>
public DateTimeOffset? AbsoluteExpiration
{
get
{
return _absoluteExpiration;
}
set
{
_absoluteExpiration = value;
}
}
public DateTimeOffset? AbsoluteExpiration { get; set; }

/// <summary>
/// Gets or sets an absolute expiration time, relative to now.
/// </summary>
public TimeSpan? AbsoluteExpirationRelativeToNow
{
get
{
return _absoluteExpirationRelativeToNow;
}
get => _absoluteExpirationRelativeToNow;
set
{
// this method does not set AbsoluteExpiration as it would require calling Clock.UtcNow twice:
// once here and once in MemoryCache.SetEntry

if (value <= TimeSpan.Zero)
{
throw new ArgumentOutOfRangeException(
Expand All @@ -111,10 +69,7 @@ public TimeSpan? AbsoluteExpirationRelativeToNow
/// </summary>
public TimeSpan? SlidingExpiration
{
get
{
return _slidingExpiration;
}
get => _slidingExpiration;
set
{
if (value <= TimeSpan.Zero)
Expand All @@ -124,41 +79,20 @@ public TimeSpan? SlidingExpiration
value,
"The sliding expiration value must be positive.");
}

_slidingExpiration = value;
}
}

/// <summary>
/// Gets the <see cref="IChangeToken"/> instances which cause the cache entry to expire.
/// </summary>
public IList<IChangeToken> ExpirationTokens
{
get
{
if (_expirationTokens == null)
{
_expirationTokens = new List<IChangeToken>();
}

return _expirationTokens;
}
}
public IList<IChangeToken> ExpirationTokens => _expirationTokens ??= new List<IChangeToken>();

/// <summary>
/// Gets or sets the callbacks will be fired after the cache entry is evicted from the cache.
/// </summary>
public IList<PostEvictionCallbackRegistration> PostEvictionCallbacks
{
get
{
if (_postEvictionCallbacks == null)
{
_postEvictionCallbacks = new List<PostEvictionCallbackRegistration>();
}

return _postEvictionCallbacks;
}
}
public IList<PostEvictionCallbackRegistration> PostEvictionCallbacks => _postEvictionCallbacks ??= new List<PostEvictionCallbackRegistration>();

/// <summary>
/// Gets or sets the priority for keeping the cache entry in the cache during a
Expand Down Expand Up @@ -191,19 +125,25 @@ public object Value
set
{
_value = value;
_valueHasBeenSet = true;
IsValueSet = true;
}
}

internal DateTimeOffset LastAccessed { get; set; }

internal EvictionReason EvictionReason { get; private set; }

private bool IsDisposed { get => ((State)_state).HasFlag(State.IsDisposed); set => Set(State.IsDisposed, value); }

private bool IsExpired { get => ((State)_state).HasFlag(State.IsExpired); set => Set(State.IsExpired, value); }

private bool IsValueSet { get => ((State)_state).HasFlag(State.IsValueSet); set => Set(State.IsValueSet, value); }

public void Dispose()
{
if (!_disposed)
if (!IsDisposed)
{
_disposed = true;
IsDisposed = true;

// Ensure the _scope reference is cleared because it can reference other CacheEntry instances.
// This CacheEntry is going to be put into a MemoryCache, and we don't want to root unnecessary objects.
Expand All @@ -213,9 +153,9 @@ public void Dispose()
// Don't commit or propagate options if the CacheEntry Value was never set.
// We assume an exception occurred causing the caller to not set the Value successfully,
// so don't use this entry.
if (_valueHasBeenSet)
if (IsValueSet)
{
_notifyCacheEntryCommit(this);
_cache.SetEntry(this);

if (CanPropagateOptions())
{
Expand All @@ -226,24 +166,21 @@ public void Dispose()
}

[MethodImpl(MethodImplOptions.AggressiveInlining)]
internal bool CheckExpired(in DateTimeOffset now)
{
return _isExpired || CheckForExpiredTime(now) || CheckForExpiredTokens();
}
internal bool CheckExpired(in DateTimeOffset now) => IsExpired || CheckForExpiredTime(now) || CheckForExpiredTokens();

internal void SetExpired(EvictionReason reason)
{
if (EvictionReason == EvictionReason.None)
{
EvictionReason = reason;
}
_isExpired = true;
IsExpired = true;
DetachTokens();
}

private bool CheckForExpiredTime(in DateTimeOffset now)
{
if (!_absoluteExpiration.HasValue && !_slidingExpiration.HasValue)
if (!AbsoluteExpiration.HasValue && !_slidingExpiration.HasValue)
{
return false;
}
Expand All @@ -252,7 +189,7 @@ private bool CheckForExpiredTime(in DateTimeOffset now)

bool FullCheck(in DateTimeOffset offset)
{
if (_absoluteExpiration.HasValue && _absoluteExpiration.Value <= offset)
if (AbsoluteExpiration.HasValue && AbsoluteExpiration.Value <= offset)
{
SetExpired(EvictionReason.Expired);
return true;
Expand Down Expand Up @@ -323,12 +260,13 @@ private static void ExpirationTokensExpired(object obj)
{
var entry = (CacheEntry)state;
entry.SetExpired(EvictionReason.TokenExpired);
entry._notifyCacheOfExpiration(entry);
entry._cache.EntryExpired(entry);
}, obj, CancellationToken.None, TaskCreationOptions.DenyChildAttach, TaskScheduler.Default);
}

private void DetachTokens()
{
// _expirationTokenRegistrations is not checked for null, because AttachTokens might initialize it under lock
lock (_lock)
{
IList<IDisposable> registrations = _expirationTokenRegistrations;
Expand Down Expand Up @@ -373,12 +311,13 @@ private static void InvokeCallbacks(CacheEntry entry)
catch (Exception e)
{
// This will be invoked on a background thread, don't let it throw.
entry._logger.LogError(e, "EvictionCallback invoked failed");
entry._cache._logger.LogError(e, "EvictionCallback invoked failed");
}
}
}

internal bool CanPropagateOptions() => _expirationTokens != null || _absoluteExpiration.HasValue;
// this simple check very often allows us to avoid expensive call to PropagateOptions(CacheEntryHelper.Current)
internal bool CanPropagateOptions() => _expirationTokens != null || AbsoluteExpiration.HasValue;

internal void PropagateOptions(CacheEntry parent)
{
Expand All @@ -403,13 +342,33 @@ internal void PropagateOptions(CacheEntry parent)
}
}

if (_absoluteExpiration.HasValue)
if (AbsoluteExpiration.HasValue)
{
if (!parent._absoluteExpiration.HasValue || _absoluteExpiration < parent._absoluteExpiration)
if (!parent.AbsoluteExpiration.HasValue || AbsoluteExpiration < parent.AbsoluteExpiration)
{
parent._absoluteExpiration = _absoluteExpiration;
parent.AbsoluteExpiration = AbsoluteExpiration;
}
}
}

private void Set(State option, bool value)
{
int before, after;

do
{
before = _state;
after = value ? (_state | (int)option) : (_state & ~(int)option);
} while (Interlocked.CompareExchange(ref _state, after, before) != before);
}

[Flags]
private enum State
{
Default = 0,
IsValueSet = 1 << 0,
IsExpired = 1 << 1,
IsDisposed = 1 << 2,
}
}
}
Loading

0 comments on commit 62b4a1f

Please sign in to comment.