Query: Use MemoryCache for RelationalCommand caching

Add test to verify the command caching

Resolves #18493

src/EFCore.Relational/Query/RelationalShapedQueryCompilingExpressionVisitor.RelationalCommandCache.cs

@@ -4,30 +4,34 @@
 using System.Collections;
 using System.Collections.Concurrent;
 using System.Collections.Generic;
-using System.Runtime.CompilerServices;
 using Microsoft.EntityFrameworkCore.Query.SqlExpressions;
 using Microsoft.EntityFrameworkCore.Storage;
+using Microsoft.Extensions.Caching.Memory;
 
 namespace Microsoft.EntityFrameworkCore.Query
 {
  public partial class RelationalShapedQueryCompilingExpressionVisitor
  {
  private class RelationalCommandCache
  {
- private readonly ConcurrentDictionary<CommandCacheKey, IRelationalCommand> _commandCache
- = new ConcurrentDictionary<CommandCacheKey, IRelationalCommand>(CommandCacheKeyComparer.Instance);
+ private static readonly ConcurrentDictionary<object, object> _syncObjects
+ = new ConcurrentDictionary<object, object>();
+ private readonly IMemoryCache _memoryCache;
+
  private readonly ISqlExpressionFactory _sqlExpressionFactory;
  private readonly IParameterNameGeneratorFactory _parameterNameGeneratorFactory;
  private readonly IQuerySqlGeneratorFactory _querySqlGeneratorFactory;
  private readonly SelectExpression _selectExpression;
  private readonly ParameterValueBasedSelectExpressionOptimizer _parameterValueBasedSelectExpressionOptimizer;
 
  public RelationalCommandCache(
+ IMemoryCache memoryCache,
  ISqlExpressionFactory sqlExpressionFactory,
  IParameterNameGeneratorFactory parameterNameGeneratorFactory,
  IQuerySqlGeneratorFactory querySqlGeneratorFactory,
  SelectExpression selectExpression)
  {
+ _memoryCache = memoryCache;
  _sqlExpressionFactory = sqlExpressionFactory;
  _parameterNameGeneratorFactory = parameterNameGeneratorFactory;
  _querySqlGeneratorFactory = querySqlGeneratorFactory;
@@ -39,43 +43,66 @@ public RelationalCommandCache(
 
  public virtual IRelationalCommand GetRelationalCommand(IReadOnlyDictionary<string, object> parameters)
  {
- var key = new CommandCacheKey(parameters);
+ var cacheKey = new CommandCacheKey(_selectExpression, parameters);
 
- if (_commandCache.TryGetValue(key, out var relationalCommand))
+ retry:
+ if (!_memoryCache.TryGetValue(cacheKey, out IRelationalCommand relationalCommand))
  {
- return relationalCommand;
- }
-
- var selectExpression = _parameterValueBasedSelectExpressionOptimizer.Optimize(_selectExpression, parameters);
+ if (!_syncObjects.TryAdd(cacheKey, value: null))
+  {
+ goto retry;
+  }
 
- relationalCommand = _querySqlGeneratorFactory.Create().GetCommand(selectExpression);
+ try
+ {
+ var selectExpression = _parameterValueBasedSelectExpressionOptimizer.Optimize(_selectExpression, parameters);
+ relationalCommand = _querySqlGeneratorFactory.Create().GetCommand(selectExpression);
 
- if (ReferenceEquals(selectExpression, _selectExpression))
- {
- _commandCache.TryAdd(key, relationalCommand);
+ if (ReferenceEquals(selectExpression, _selectExpression))
+ {
+ _memoryCache.Set(cacheKey, relationalCommand, new MemoryCacheEntryOptions { Size = 10 });
+ }
+ }
+ finally
+ {
+ _syncObjects.TryRemove(cacheKey, out _);
+ }
  }
 
  return relationalCommand;
  }
 
- private sealed class CommandCacheKeyComparer : IEqualityComparer<CommandCacheKey>
+ private readonly struct CommandCacheKey
  {
- public static readonly CommandCacheKeyComparer Instance = new CommandCacheKeyComparer();
+ public readonly SelectExpression _selectExpression;
+ public readonly IReadOnlyDictionary<string, object> _parameterValues;
 
- private CommandCacheKeyComparer()
+ public CommandCacheKey(SelectExpression selectExpression, IReadOnlyDictionary<string, object> parameterValues)
  {
+ _selectExpression = selectExpression;
+ _parameterValues = parameterValues;
  }
 
- [MethodImpl(MethodImplOptions.AggressiveInlining)]
- public bool Equals(CommandCacheKey x, CommandCacheKey y)
+ public override bool Equals(object obj)
+ => obj != null
+ && (ReferenceEquals(this, obj)
+ || obj is CommandCacheKey commandCacheKey
+ && Equals(commandCacheKey));
+
+ private bool Equals(CommandCacheKey commandCacheKey)
  {
- if (x.ParameterValues.Count > 0)
+ if (!ReferenceEquals(_selectExpression, commandCacheKey._selectExpression))
+ {
+ return false;
+ }
+
+ if (_parameterValues.Count > 0)
  {
- foreach (var parameterValue in x.ParameterValues)
+ foreach (var parameterValue in _parameterValues)
  {
  var value = parameterValue.Value;
 
- if (!y.ParameterValues.TryGetValue(parameterValue.Key, out var otherValue))
+ if (!commandCacheKey._parameterValues.TryGetValue(parameterValue.Key, out var otherValue))
  {
  return false;
  }
@@ -98,16 +125,7 @@ public bool Equals(CommandCacheKey x, CommandCacheKey y)
  return true;
  }
 
- [MethodImpl(MethodImplOptions.AggressiveInlining)]
- public int GetHashCode(CommandCacheKey obj) => 0;
- }
-
- private readonly struct CommandCacheKey
- {
- public readonly IReadOnlyDictionary<string, object> ParameterValues;
-
- public CommandCacheKey(IReadOnlyDictionary<string, object> parameterValues)
- => ParameterValues = parameterValues;
+ public override int GetHashCode() => 0;
  }
  }
  }

src/EFCore.Relational/Query/RelationalShapedQueryCompilingExpressionVisitor.cs

@@ -64,6 +64,7 @@ protected override Expression VisitShapedQueryExpression(ShapedQueryExpression s
  }
 
  var relationalCommandCache = new RelationalCommandCache(
+ Dependencies.MemoryCache,
  RelationalDependencies.SqlExpressionFactory,
  RelationalDependencies.ParameterNameGeneratorFactory,
  RelationalDependencies.QuerySqlGeneratorFactory,

src/EFCore/Query/ShapedQueryCompilingExpressionVisitorDependencies.cs

@@ -5,6 +5,7 @@
 using Microsoft.EntityFrameworkCore.Infrastructure;
 using Microsoft.EntityFrameworkCore.Storage;
 using Microsoft.EntityFrameworkCore.Utilities;
+using Microsoft.Extensions.Caching.Memory;
 using Microsoft.Extensions.DependencyInjection;
 
 namespace Microsoft.EntityFrameworkCore.Query
@@ -56,13 +57,16 @@ public sealed class ShapedQueryCompilingExpressionVisitorDependencies
  [EntityFrameworkInternal]
  public ShapedQueryCompilingExpressionVisitorDependencies(
  [NotNull] IEntityMaterializerSource entityMaterializerSource,
- [NotNull] ITypeMappingSource typeMappingSource)
+ [NotNull] ITypeMappingSource typeMappingSource,
+ [NotNull] IMemoryCache memoryCache)
  {
  Check.NotNull(entityMaterializerSource, nameof(entityMaterializerSource));
  Check.NotNull(typeMappingSource, nameof(typeMappingSource));
+ Check.NotNull(memoryCache, nameof(memoryCache));
 
  EntityMaterializerSource = entityMaterializerSource;
  TypeMappingSource = typeMappingSource;
+ MemoryCache = memoryCache;
  }
 
  /// <summary>
@@ -75,20 +79,33 @@ public ShapedQueryCompilingExpressionVisitorDependencies(
  /// </summary>
  public ITypeMappingSource TypeMappingSource { get; }
 
+ /// <summary>
+ /// The memory cache.
+ /// </summary>
+ public IMemoryCache MemoryCache { get; }
+
  /// <summary>
  /// Clones this dependency parameter object with one service replaced.
  /// </summary>
  /// <param name="entityMaterializerSource"> A replacement for the current dependency of this type. </param>
  /// <returns> A new parameter object with the given service replaced. </returns>
  public ShapedQueryCompilingExpressionVisitorDependencies With([NotNull] IEntityMaterializerSource entityMaterializerSource)
- => new ShapedQueryCompilingExpressionVisitorDependencies(entityMaterializerSource, TypeMappingSource);
+ => new ShapedQueryCompilingExpressionVisitorDependencies(entityMaterializerSource, TypeMappingSource, MemoryCache);
 
  /// <summary>
  /// Clones this dependency parameter object with one service replaced.
  /// </summary>
  /// <param name="typeMappingSource"> A replacement for the current dependency of this type. </param>
  /// <returns> A new parameter object with the given service replaced. </returns>
  public ShapedQueryCompilingExpressionVisitorDependencies With([NotNull] ITypeMappingSource typeMappingSource)
- => new ShapedQueryCompilingExpressionVisitorDependencies(EntityMaterializerSource, typeMappingSource);
+ => new ShapedQueryCompilingExpressionVisitorDependencies(EntityMaterializerSource, typeMappingSource, MemoryCache);
+
+ /// <summary>
+ /// Clones this dependency parameter object with one service replaced.
+ /// </summary>
+ /// <param name="memoryCache"> A replacement for the current dependency of this type. </param>
+ /// <returns> A new parameter object with the given service replaced. </returns>
+ public ShapedQueryCompilingExpressionVisitorDependencies With([NotNull] IMemoryCache memoryCache)
+ => new ShapedQueryCompilingExpressionVisitorDependencies(EntityMaterializerSource, TypeMappingSource, memoryCache);
  }
 }

test/EFCore.SqlServer.FunctionalTests/Query/QueryBugsTest.cs

@@ -2243,11 +2243,11 @@ public virtual void Variable_from_closure_is_parametrized()
 
  var id = 1;
  context.Entities.Where(c => c.Id == id).ToList();
- Assert.Equal(1, context.Cache.Count);
+ Assert.Equal(2, context.Cache.Count);
 
  id = 2;
  context.Entities.Where(c => c.Id == id).ToList();
- Assert.Equal(1, context.Cache.Count);
+ Assert.Equal(2, context.Cache.Count);
 
  AssertSql(
  @"@__id_0='1'
@@ -2280,11 +2280,11 @@ public virtual void Variable_from_nested_closure_is_parametrized()
 
  id = 1;
  context.Entities.Where(whereExpression).ToList();
- Assert.Equal(1, context.Cache.Count);
+ Assert.Equal(2, context.Cache.Count);
 
  id = 2;
  context.Entities.Where(whereExpression).ToList();
- Assert.Equal(1, context.Cache.Count);
+ Assert.Equal(2, context.Cache.Count);
 
  AssertSql(
  @"@__id_0='1'
@@ -2319,11 +2319,11 @@ public virtual void Variable_from_multi_level_nested_closure_is_parametrized()
 
  id = 1;
  context.Entities.Where(containsExpression).ToList();
- Assert.Equal(1, context.Cache.Count);
+ Assert.Equal(2, context.Cache.Count);
 
  id = 2;
  context.Entities.Where(containsExpression).ToList();
- Assert.Equal(1, context.Cache.Count);
+ Assert.Equal(2, context.Cache.Count);
 
  AssertSql(
  @"@__id_0='1'
@@ -2349,6 +2349,40 @@ FROM [Entities] AS [e0]
  }
  }
 
+ [ConditionalFact]
+ public virtual void Relational_command_cache_creates_new_entry_when_parameter_nullability_changes()
+ {
+ using (CreateDatabase8909())
+ {
+ using (var context = new MyContext8909(_options))
+ {
+ context.Cache.Compact(1);
+
+ var name = "A";
+
+ context.Entities.Where(e => e.Name == name).ToList();
+ Assert.Equal(2, context.Cache.Count);
+
+ name = null;
+ context.Entities.Where(e => e.Name == name).ToList();
+ Assert.Equal(3, context.Cache.Count);
+
+ AssertSql(
+ @"@__name_0='A' (Size = 4000)
+
+SELECT [e].[Id], [e].[Name]
+FROM [Entities] AS [e]
+WHERE (([e].[Name] = @__name_0) AND ([e].[Name] IS NOT NULL AND @__name_0 IS NOT NULL)) OR ([e].[Name] IS NULL AND @__name_0 IS NULL)",
+ //
+ @"@__name_0=NULL (Size = 4000)
+
+SELECT [e].[Id], [e].[Name]
+FROM [Entities] AS [e]
+WHERE (([e].[Name] = @__name_0) AND ([e].[Name] IS NOT NULL AND @__name_0 IS NOT NULL)) OR ([e].[Name] IS NULL AND @__name_0 IS NULL)");
+ }
+ }
+ }
+
  [ConditionalFact]
  public virtual void Query_cache_entries_are_evicted_as_necessary()
  {