Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reduce CacheEntry size #45962

Merged
merged 19 commits into from
Jan 7, 2021
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
227c96d
reduce the size of CacheEntry instance by 2xsizeof(int)
adamsitnik Dec 10, 2020
5c8b862
remove _lock field and use "this" for locking instead
adamsitnik Dec 10, 2020
4b529fb
move _expirationTokenRegistrations, _postEvictionCallbacks and _expir…
adamsitnik Dec 10, 2020
c04da17
move private types into separate files
adamsitnik Dec 10, 2020
217ab43
rename the types + add comments
adamsitnik Dec 10, 2020
c6597db
inline private properties that are used just twice in total
adamsitnik Dec 11, 2020
c519666
force inline some methods based on profiling data
adamsitnik Dec 11, 2020
4a5715a
rename files
adamsitnik Dec 11, 2020
f170e8d
apply code review suggestion
adamsitnik Dec 11, 2020
01a2686
apply code review suggestion: don't use Enum.HasFlag
adamsitnik Dec 11, 2020
0c07731
apply code review suggestion: don't use Enum.HasFlag
adamsitnik Dec 11, 2020
46ae2ea
apply code review suggestion: remove redundant Interlocked.CompareExc…
adamsitnik Dec 11, 2020
c3e75ea
address code review feedback: encapsulate the fields of CacheEntryTokens
adamsitnik Dec 14, 2020
f582eb6
Apply suggestions from code review
adamsitnik Jan 5, 2021
6a39239
there is no need for explicit memory layout anymore
adamsitnik Jan 5, 2021
58d6a0e
there is no need for using IList abstraction (the field was changed f…
adamsitnik Jan 5, 2021
a1fcbd1
restore call to parameterless ctor to ensure that all struct fields a…
adamsitnik Jan 5, 2021
356e02f
CopyTokens => PropagateTokens for naming consistency with PropagateOp…
adamsitnik Jan 5, 2021
9b189eb
Apply suggestions from code review
adamsitnik Jan 7, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
151 changes: 32 additions & 119 deletions src/libraries/Microsoft.Extensions.Caching.Memory/src/CacheEntry.cs
Original file line number Diff line number Diff line change
Expand Up @@ -11,28 +11,26 @@

namespace Microsoft.Extensions.Caching.Memory
{
internal class CacheEntry : ICacheEntry
internal partial class CacheEntry : ICacheEntry
{
private static readonly Action<object> ExpirationCallback = ExpirationTokensExpired;

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

private IList<IDisposable> _expirationTokenRegistrations;
private IList<PostEvictionCallbackRegistration> _postEvictionCallbacks;
private IList<IChangeToken> _expirationTokens;
private CacheEntryTokens _tokens; // might be null if user is not using the tokens or callbacks
adamsitnik marked this conversation as resolved.
Show resolved Hide resolved
private TimeSpan? _absoluteExpirationRelativeToNow;
private TimeSpan? _slidingExpiration;
private long? _size;
private CacheEntry _previous; // this field is not null only before the entry is added to the cache
private object _value;
private int _state; // actually a [Flag] enum called "State"
private CacheEntryState _state;

internal CacheEntry(object key, MemoryCache memoryCache)
{
Key = key ?? throw new ArgumentNullException(nameof(key));
_cache = memoryCache ?? throw new ArgumentNullException(nameof(memoryCache));
_previous = CacheEntryHelper.EnterScope(this);
_state = new CacheEntryState(CacheItemPriority.Normal);
}

/// <summary>
Expand Down Expand Up @@ -87,18 +85,18 @@ public TimeSpan? SlidingExpiration
/// <summary>
/// Gets the <see cref="IChangeToken"/> instances which cause the cache entry to expire.
/// </summary>
public IList<IChangeToken> ExpirationTokens => _expirationTokens ??= new List<IChangeToken>();
public IList<IChangeToken> ExpirationTokens => GetOrCreateTokens().ExpirationTokens;

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

/// <summary>
/// Gets or sets the priority for keeping the cache entry in the cache during a
/// memory pressure triggered cleanup. The default is <see cref="CacheItemPriority.Normal"/>.
/// </summary>
public CacheItemPriority Priority { get; set; } = CacheItemPriority.Normal;
public CacheItemPriority Priority { get => _state.Priority; set => _state.Priority = value; }

/// <summary>
/// Gets or sets the size of the cache entry value.
Expand All @@ -125,30 +123,24 @@ public object Value
set
{
_value = value;
IsValueSet = true;
_state.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); }
internal EvictionReason EvictionReason { get => _state.EvictionReason; private set => _state.EvictionReason = value; }

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

// 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 (IsValueSet)
if (_state.IsValueSet)
{
_cache.SetEntry(this);

Expand All @@ -163,19 +155,23 @@ public void Dispose()
}
}

[MethodImpl(MethodImplOptions.AggressiveInlining)]
internal bool CheckExpired(in DateTimeOffset now) => IsExpired || CheckForExpiredTime(now) || CheckForExpiredTokens();
[MethodImpl(MethodImplOptions.AggressiveInlining)] // added based on profiling
internal bool CheckExpired(in DateTimeOffset now)
=> _state.IsExpired
|| CheckForExpiredTime(now)
|| (_tokens != null && _tokens.CheckForExpiredTokens(this));

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

[MethodImpl(MethodImplOptions.AggressiveInlining)] // added based on profiling
private bool CheckForExpiredTime(in DateTimeOffset now)
{
if (!AbsoluteExpiration.HasValue && !_slidingExpiration.HasValue)
Expand Down Expand Up @@ -204,52 +200,7 @@ bool FullCheck(in DateTimeOffset offset)
}
}

private bool CheckForExpiredTokens()
{
if (_expirationTokens == null)
{
return false;
}

return CheckTokens();

bool CheckTokens()
{
for (int i = 0; i < _expirationTokens.Count; i++)
{
IChangeToken expiredToken = _expirationTokens[i];
if (expiredToken.HasChanged)
{
SetExpired(EvictionReason.TokenExpired);
return true;
}
}
return false;
}
}

internal void AttachTokens()
{
if (_expirationTokens != null)
{
lock (_lock)
{
for (int i = 0; i < _expirationTokens.Count; i++)
{
IChangeToken expirationToken = _expirationTokens[i];
if (expirationToken.ActiveChangeCallbacks)
{
if (_expirationTokenRegistrations == null)
{
_expirationTokenRegistrations = new List<IDisposable>(1);
}
IDisposable registration = expirationToken.RegisterChangeCallback(ExpirationCallback, this);
_expirationTokenRegistrations.Add(registration);
}
}
}
}
}
internal void AttachTokens() => _tokens?.AttachTokens();

private static void ExpirationTokensExpired(object obj)
{
Expand All @@ -262,27 +213,9 @@ private static void ExpirationTokensExpired(object obj)
}, 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;
if (registrations != null)
{
_expirationTokenRegistrations = null;
for (int i = 0; i < registrations.Count; i++)
{
IDisposable registration = registrations[i];
registration.Dispose();
}
}
}
}

internal void InvokeEvictionCallbacks()
{
if (_postEvictionCallbacks != null)
if (_tokens?._postEvictionCallbacks != null)
{
Task.Factory.StartNew(state => InvokeCallbacks((CacheEntry)state), this,
CancellationToken.None, TaskCreationOptions.DenyChildAttach, TaskScheduler.Default);
Expand All @@ -291,7 +224,7 @@ internal void InvokeEvictionCallbacks()

private static void InvokeCallbacks(CacheEntry entry)
{
IList<PostEvictionCallbackRegistration> callbackRegistrations = Interlocked.Exchange(ref entry._postEvictionCallbacks, null);
IList<PostEvictionCallbackRegistration> callbackRegistrations = Interlocked.Exchange(ref entry._tokens._postEvictionCallbacks, null);

if (callbackRegistrations == null)
{
Expand All @@ -315,7 +248,8 @@ private static void InvokeCallbacks(CacheEntry entry)
}

// this simple check very often allows us to avoid expensive call to PropagateOptions(CacheEntryHelper.Current)
internal bool CanPropagateOptions() => _expirationTokens != null || AbsoluteExpiration.HasValue;
[MethodImpl(MethodImplOptions.AggressiveInlining)] // added based on profiling
internal bool CanPropagateOptions() => _tokens?._expirationTokens != null || AbsoluteExpiration.HasValue;

internal void PropagateOptions(CacheEntry parent)
{
Expand All @@ -326,19 +260,7 @@ internal void PropagateOptions(CacheEntry parent)

// Copy expiration tokens and AbsoluteExpiration to the cache entries hierarchy.
// We do this regardless of it gets cached because the tokens are associated with the value we'll return.
if (_expirationTokens != null)
{
lock (_lock)
{
lock (parent._lock)
{
foreach (IChangeToken expirationToken in _expirationTokens)
{
parent.AddExpirationToken(expirationToken);
}
}
}
}
_tokens?.CopyTokens(parent);

if (AbsoluteExpiration.HasValue)
{
Expand All @@ -349,24 +271,15 @@ internal void PropagateOptions(CacheEntry parent)
}
}

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

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

[Flags]
private enum State
{
Default = 0,
IsValueSet = 1 << 0,
IsExpired = 1 << 1,
IsDisposed = 1 << 2,
CacheEntryTokens result = new CacheEntryTokens();
return Interlocked.CompareExchange(ref _tokens, result, null) ?? result;
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,95 @@
// Licensed to the .NET Foundation under one or more agreements.
adamsitnik marked this conversation as resolved.
Show resolved Hide resolved
// The .NET Foundation licenses this file to you under the MIT license.

using System;
using System.Runtime.CompilerServices;
using System.Runtime.InteropServices;
using System.Threading;

namespace Microsoft.Extensions.Caching.Memory
{
internal partial class CacheEntry
{
// this type exists just to reduce CacheEntry size by replacing many enum & boolean fields with one of a size of Int32
[StructLayout(LayoutKind.Explicit)]
private struct CacheEntryState
{
// this field overlaps with the 4 other byte fields, it exists to allow for atomic updates
[FieldOffset(0)]
private int _state;

[FieldOffset(0)]
private byte _flags;
[FieldOffset(1)]
private byte _evictionReason;
[FieldOffset(2)]
private byte _priority;
[FieldOffset(3)]
private byte _reserved; // for future use

internal CacheEntryState(CacheItemPriority priority) : this() => _priority = (byte)priority;

internal bool IsDisposed { get => ((Flags)_flags).HasFlag(Flags.IsDisposed); set => SetFlag(Flags.IsDisposed, value); }

internal bool IsExpired
{
[MethodImpl(MethodImplOptions.AggressiveInlining)] // added based on profiling
get => ((Flags)_flags).HasFlag(Flags.IsExpired);
set => SetFlag(Flags.IsExpired, value);
}

internal bool IsValueSet { get => ((Flags)_flags).HasFlag(Flags.IsValueSet); set => SetFlag(Flags.IsValueSet, value); }

internal EvictionReason EvictionReason
{
get => (EvictionReason)_evictionReason;
set
{
CacheEntryState before, after;
do
{
before = this;
after = this;
after._evictionReason = (byte)value;
} while (Interlocked.CompareExchange(ref _state, after._state, before._state) != before._state);
}
}

internal CacheItemPriority Priority
{
get => (CacheItemPriority)_priority;
set
{
CacheEntryState before, after;
do
{
before = this;
after = this;
after._priority = (byte)value;
} while (Interlocked.CompareExchange(ref _state, after._state, before._state) != before._state);
}
}

private void SetFlag(Flags option, bool value)
{
CacheEntryState before, after;

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

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