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

Introduce Sliding Cache to Constant Expression Helper #765

Merged
merged 11 commits into from
Jan 22, 2024
23 changes: 14 additions & 9 deletions src/System.Linq.Dynamic.Core/Parser/ConstantExpressionHelper.cs
Original file line number Diff line number Diff line change
@@ -1,12 +1,15 @@
using System.Collections.Concurrent;
using System.Linq.Dynamic.Core.Util;
using System.Linq.Expressions;

namespace System.Linq.Dynamic.Core.Parser
{
internal static class ConstantExpressionHelper
{
private static readonly ConcurrentDictionary<object, Expression> Expressions = new();
private static readonly ConcurrentDictionary<Expression, string> Literals = new();
private static readonly TimeSpan TimeToLivePeriod = TimeSpan.FromMinutes(10);

public static readonly ThreadSafeSlidingCache<object, Expression> Expressions = new(TimeToLivePeriod);
private static readonly ThreadSafeSlidingCache<Expression, string> Literals = new(TimeToLivePeriod);


public static bool TryGetText(Expression expression, out string? text)
{
Expand All @@ -15,15 +18,17 @@ public static bool TryGetText(Expression expression, out string? text)

public static Expression CreateLiteral(object value, string text)
{
if (!Expressions.ContainsKey(value))
if (Expressions.TryGetValue(value, out var outputValue))
{
ConstantExpression constantExpression = Expression.Constant(value);

Expressions.TryAdd(value, constantExpression);
Literals.TryAdd(constantExpression, text);
return outputValue;
}

return Expressions[value];
ConstantExpression constantExpression = Expression.Constant(value);

Expressions.AddOrUpdate(value, constantExpression);
Literals.AddOrUpdate(constantExpression, text);

return constantExpression;
}
}
}
12 changes: 12 additions & 0 deletions src/System.Linq.Dynamic.Core/Util/DateTimeUtils.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
namespace System.Linq.Dynamic.Core.Util
{
internal interface IDateTimeUtils
{
DateTime UtcNow { get; }
}

internal class DateTimeUtils : IDateTimeUtils
{
public DateTime UtcNow => DateTime.UtcNow;
}
}
135 changes: 135 additions & 0 deletions src/System.Linq.Dynamic.Core/Util/ThreadSafeSlidingCache.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,135 @@
using System.Collections.Concurrent;
using System.Linq.Dynamic.Core.Validation;
using System.Threading.Tasks;

namespace System.Linq.Dynamic.Core.Util
{
internal static class ThreadSafeSlidingCacheConstants
{
// Default cleanup frequency
public static readonly TimeSpan DefaultCleanupFrequency = TimeSpan.FromMinutes(10);
}

internal class ThreadSafeSlidingCache<TKey, TValue> where TKey : notnull where TValue : notnull
{
private readonly ConcurrentDictionary<TKey, (TValue Value, DateTime ExpirationTime)> _cache;
private readonly TimeSpan _cleanupFrequency;
private readonly IDateTimeUtils _dateTimeProvider;
private readonly Func<Task> _deleteExpiredCachedItemsDelegate;
private readonly long? _minCacheItemsBeforeCleanup;
private DateTime _lastCleanupTime = DateTime.MinValue;

/// <summary>
/// Sliding Thread Safe Cache
/// </summary>
/// <param name="timeToLive">The length of time any object would survive before being removed</param>
/// <param name="cleanupFrequency">Only look for expired objects over specific periods</param>
/// <param name="minCacheItemsBeforeCleanup">
/// If defined, only allow the cleanup process after x number of cached items have
/// been stored
/// </param>
/// <param name="dateTimeProvider">
/// Provides the Time for the Caching object. Default will be created if not supplied. Used
/// for Testing classes
/// </param>
public ThreadSafeSlidingCache(
TimeSpan timeToLive,
TimeSpan? cleanupFrequency = null,
long? minCacheItemsBeforeCleanup = null,
IDateTimeUtils? dateTimeProvider = null)
{
_cache = new ConcurrentDictionary<TKey, (TValue, DateTime)>();
TimeToLive = timeToLive;
_minCacheItemsBeforeCleanup = minCacheItemsBeforeCleanup;
_cleanupFrequency = cleanupFrequency ?? ThreadSafeSlidingCacheConstants.DefaultCleanupFrequency;
_deleteExpiredCachedItemsDelegate = Cleanup;
_dateTimeProvider = dateTimeProvider ?? new DateTimeUtils();
}

public TimeSpan TimeToLive { get; }

/// <summary>
/// Provide the number of items in the cache
/// </summary>
public int Count => _cache.Count;

public void AddOrUpdate(TKey key, TValue value)
{
Check.NotNull(key);
Check.NotNull(value);

var expirationTime = _dateTimeProvider.UtcNow.Add(TimeToLive);
_cache[key] = (value, expirationTime);

CleanupIfNeeded();
}

public bool TryGetValue(TKey key, out TValue value)
{
Check.NotNull(key);

CleanupIfNeeded();

if (_cache.TryGetValue(key, out var valueAndExpiration))
{
if (_dateTimeProvider.UtcNow <= valueAndExpiration.ExpirationTime)
{
value = valueAndExpiration.Value;
_cache[key] = (value, _dateTimeProvider.UtcNow.Add(TimeToLive));
return true;
}

// Remove expired item
_cache.TryRemove(key, out _);
}

value = default!;
return false;
}

public bool Remove(TKey key)
{
Check.NotNull(key);

var removed = _cache.TryRemove(key, out _);
CleanupIfNeeded();
return removed;
}

/// <summary>
/// Check if cache needs to be cleaned up.
/// If it does, span the cleanup as a Task to prevent from blocking
/// </summary>
private void CleanupIfNeeded()
{
if (_dateTimeProvider.UtcNow - _lastCleanupTime > _cleanupFrequency
&& (_minCacheItemsBeforeCleanup == null ||
_cache.Count >=
_minCacheItemsBeforeCleanup) // Only cleanup if we have a minimum number of items in the cache.
)
{
// Set here, so we don't have re-entry due to large collection enumeration.
_lastCleanupTime = _dateTimeProvider.UtcNow;

Task.Run(_deleteExpiredCachedItemsDelegate);
}
}

/// <summary>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

private methods do not need xml-comment, expect when very special or complex

/// Cleanup the Cache
/// </summary>
/// <returns></returns>
private Task Cleanup()
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rename to CleanupAsync

{
foreach (var key in _cache.Keys)
{
if (_dateTimeProvider.UtcNow > _cache[key].ExpirationTime)
{
_cache.TryRemove(key, out _);
}
}

return Task.CompletedTask;
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
using System.Linq.Dynamic.Core.Parser;
using System.Threading.Tasks;
using Xunit;

namespace System.Linq.Dynamic.Core.Tests
{
public partial class EntitiesTests
{
//[Fact]
//public async Task TestConstantExpressionLeak()
//{
// //Arrange
// PopulateTestData(1, 0);

// var populateExpression = _context.Blogs.All("BlogId > 2000");

// var expressions = ConstantExpressionHelper.Expressions;

// // Should contain
// if (!expressions.TryGetValue(2000, out _))
// {
// Assert.Fail("Cache was missing constant expression for 2000");
// }

// // wait half the expiry time
// await Task.Delay(TimeSpan.FromSeconds(ConstantExpressionHelper.Expressions.TimeToLive.TotalSeconds/2));
// if (!expressions.TryGetValue(2000, out _))
// {
// Assert.Fail("Cache was missing constant expression for 2000 (1)");
// }

// // wait another half the expiry time, plus one second
// await Task.Delay(TimeSpan.FromSeconds((ConstantExpressionHelper.Expressions.TimeToLive.TotalSeconds / 2)+1));
// if (!expressions.TryGetValue(2000, out _))
// {
// Assert.Fail("Cache was missing constant expression for 2000 (2)");
// }

// // Wait for the slide cache to expire, check on second later
// await Task.Delay(ConstantExpressionHelper.Expressions.TimeToLive.Add(TimeSpan.FromSeconds(1)));

// if (expressions.TryGetValue(2000, out _))
// {
// Assert.Fail("Expected constant to be expired 2000");
// }
//}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,152 @@
using System.Linq.Dynamic.Core.Util;
using System.Linq.Expressions;
using System.Threading.Tasks;
using Xunit;

namespace System.Linq.Dynamic.Core.Tests.Util
{
public class ThreadSafeSlidingCacheTests
{
[Fact]
public void ThreadSafeSlidingCache_CacheOperations()
{
var mockDateTime = new MockDateTimeProvider();

// Arrange
var cache = new ThreadSafeSlidingCache<int, string>(
TimeSpan.FromSeconds(1),
dateTimeProvider: mockDateTime);

// Add
cache.AddOrUpdate(1, "one");
cache.AddOrUpdate(2, "two");
cache.AddOrUpdate(3, "three");

// Replace
cache.AddOrUpdate(1, "oneone");

Assert.True(cache.Count == 3, $"Expected 3 items in the cache, only had {cache.Count}");


// Test retrieval
Assert.True(cache.TryGetValue(1, out var value1), $"Expected to find the value, but did not");
Assert.True(cache.TryGetValue(2, out var value2), $"Expected to find the value, but did not");
Assert.True(cache.TryGetValue(3, out var value3), $"Expected to find the value, but did not");

// Test Removal
cache.Remove(1);
Assert.True(cache.Count == 2, $"Expected 2 items in the cache, only had {cache.Count}");

}

[Fact]
public void ThreadSafeSlidingCache_TestExpire()
{
var mockDateTime = new MockDateTimeProvider();

// Arrange
var cache = new ThreadSafeSlidingCache<int, string>(TimeSpan.FromMinutes(10),
dateTimeProvider: mockDateTime);

// Act
cache.AddOrUpdate(1, "one");
mockDateTime.UtcNow = mockDateTime.UtcNow.AddMinutes(11);
if (cache.TryGetValue(1, out var value))
{
Assert.True(false, $"Expected to not find the value, but found {value}");
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In this case, use Asser.Fail

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lol, that's a Co-pilot fail! Fixed! How did I miss that!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, going back to this - having trouble with xUnit - does not appear they have Assert.Fail(..) - I think Co-pilot suggested that as an alternative. Some StackOverflow articles show I should just throw an exception instead, which will fail it. I am not sure how I feel about that. How would you want me to trigger a failure?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I understand. I was using an older version from xunit in the solution.
It's added here: xunit/xunit#2609 (comment)

If you want; you upgrade all unit-test projects to use 2.5.0 which includes Assert.Fail

Else if this too much work, just keep the existing code.

}

}

[Fact]
public async Task ThreadSafeSlidingCache_TestAutoExpire()
{
var mockDateTime = new MockDateTimeProvider();

// Arrange
var cache = new ThreadSafeSlidingCache<int, string>(TimeSpan.FromMinutes(10),
dateTimeProvider: mockDateTime);

// Act
cache.AddOrUpdate(1, "one");

// Ensure one item is in the cache
Assert.True(cache.Count == 1, $"Expected 1 items in the cache, only had {cache.Count}");

// move the time forward
mockDateTime.UtcNow = mockDateTime.UtcNow.AddMinutes(11);

// Trigger the cleanup, asking for non-existing key
cache.TryGetValue(10, out var _);

// Since the cache cleanup is triggered by a Task and not on the same thread,
// give it a moment for the cleanup to happen
await Task.Delay(10);

// Ensure one item is in the cache
Assert.True(cache.Count == 0, $"Expected 0 items in the cache, only had {cache.Count}");

}

[Fact]
public async Task ThreadSafeSlidingCache_TestNull()
{
// Arrange
var cache = new ThreadSafeSlidingCache<Expression, string>(TimeSpan.FromMinutes(10));

// Expect an ArgumentNullException
var exception = Assert.Throws<ArgumentNullException>(() => {
cache.AddOrUpdate(null, "one");
});

}

[Fact]
public async Task ThreadSafeSlidingCache_TestMinNumberBeforeTests()
{
// Arrange
var mockDateTime = new MockDateTimeProvider();

// Arrange
var cache = new ThreadSafeSlidingCache<int, string>(
TimeSpan.FromMinutes(10),
minCacheItemsBeforeCleanup: 2,
dateTimeProvider: mockDateTime);

// Act
cache.AddOrUpdate(1, "one");

// Ensure one item is in the cache
Assert.True(cache.Count == 1, $"Expected 1 items in the cache, only had {cache.Count}");

// move the time forward
mockDateTime.UtcNow = mockDateTime.UtcNow.AddMinutes(11);

// Trigger the cleanup, asking for non-existing key
cache.TryGetValue(10, out var _);

// Since the cache cleanup is triggered by a Task and not on the same thread,
// give it a moment for the cleanup to happen
await Task.Delay(10);

// Ensure one item is in the cache
Assert.True(cache.Count == 1, $"Expected 1 items in the cache, only had {cache.Count}");

// Act
cache.AddOrUpdate(2, "two");

// Since the cache cleanup is triggered by a Task and not on the same thread,
// give it a moment for the cleanup to happen
await Task.Delay(10);

// Ensure one item is in the cache
Assert.True(cache.Count == 1, $"Expected 1 items in the cache, had {cache.Count}");
}

private class MockDateTimeProvider : IDateTimeUtils
{
public DateTime UtcNow { get; set; } = DateTime.UtcNow;

}
}
}