From 1caca3e5dd501b4356e6b583b2c71cbe43dba0ed Mon Sep 17 00:00:00 2001 From: Smit Patel Date: Mon, 21 Oct 2019 17:01:23 -0700 Subject: [PATCH] Query: Use MemoryCache for RelationalCommand caching Add test to verify the command caching Resolves #18493 --- ...xpressionVisitor.RelationalCommandCache.cs | 80 ++++++++++++------- ...alShapedQueryCompilingExpressionVisitor.cs | 1 + ...yCompilingExpressionVisitorDependencies.cs | 23 +++++- .../Query/QueryBugsTest.cs | 46 +++++++++-- 4 files changed, 110 insertions(+), 40 deletions(-) diff --git a/src/EFCore.Relational/Query/RelationalShapedQueryCompilingExpressionVisitor.RelationalCommandCache.cs b/src/EFCore.Relational/Query/RelationalShapedQueryCompilingExpressionVisitor.RelationalCommandCache.cs index 5d9bb1577c7..02d52de840f 100644 --- a/src/EFCore.Relational/Query/RelationalShapedQueryCompilingExpressionVisitor.RelationalCommandCache.cs +++ b/src/EFCore.Relational/Query/RelationalShapedQueryCompilingExpressionVisitor.RelationalCommandCache.cs @@ -4,9 +4,9 @@ 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 { @@ -14,8 +14,10 @@ public partial class RelationalShapedQueryCompilingExpressionVisitor { private class RelationalCommandCache { - private readonly ConcurrentDictionary _commandCache - = new ConcurrentDictionary(CommandCacheKeyComparer.Instance); + private static readonly ConcurrentDictionary _syncObjects + = new ConcurrentDictionary(); + private readonly IMemoryCache _memoryCache; + private readonly ISqlExpressionFactory _sqlExpressionFactory; private readonly IParameterNameGeneratorFactory _parameterNameGeneratorFactory; private readonly IQuerySqlGeneratorFactory _querySqlGeneratorFactory; @@ -23,11 +25,13 @@ private readonly ConcurrentDictionary _comm 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 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 + private readonly struct CommandCacheKey { - public static readonly CommandCacheKeyComparer Instance = new CommandCacheKeyComparer(); + public readonly SelectExpression _selectExpression; + public readonly IReadOnlyDictionary _parameterValues; - private CommandCacheKeyComparer() + public CommandCacheKey(SelectExpression selectExpression, IReadOnlyDictionary 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 ParameterValues; - - public CommandCacheKey(IReadOnlyDictionary parameterValues) - => ParameterValues = parameterValues; + public override int GetHashCode() => 0; } } } diff --git a/src/EFCore.Relational/Query/RelationalShapedQueryCompilingExpressionVisitor.cs b/src/EFCore.Relational/Query/RelationalShapedQueryCompilingExpressionVisitor.cs index 323b0fedf35..fd5d35574e5 100644 --- a/src/EFCore.Relational/Query/RelationalShapedQueryCompilingExpressionVisitor.cs +++ b/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, diff --git a/src/EFCore/Query/ShapedQueryCompilingExpressionVisitorDependencies.cs b/src/EFCore/Query/ShapedQueryCompilingExpressionVisitorDependencies.cs index 9f96d31a98f..5c9f248d7bc 100644 --- a/src/EFCore/Query/ShapedQueryCompilingExpressionVisitorDependencies.cs +++ b/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; } /// @@ -75,13 +79,18 @@ public ShapedQueryCompilingExpressionVisitorDependencies( /// public ITypeMappingSource TypeMappingSource { get; } + /// + /// The memory cache. + /// + public IMemoryCache MemoryCache { get; } + /// /// Clones this dependency parameter object with one service replaced. /// /// A replacement for the current dependency of this type. /// A new parameter object with the given service replaced. public ShapedQueryCompilingExpressionVisitorDependencies With([NotNull] IEntityMaterializerSource entityMaterializerSource) - => new ShapedQueryCompilingExpressionVisitorDependencies(entityMaterializerSource, TypeMappingSource); + => new ShapedQueryCompilingExpressionVisitorDependencies(entityMaterializerSource, TypeMappingSource, MemoryCache); /// /// Clones this dependency parameter object with one service replaced. @@ -89,6 +98,14 @@ public ShapedQueryCompilingExpressionVisitorDependencies With([NotNull] IEntityM /// A replacement for the current dependency of this type. /// A new parameter object with the given service replaced. public ShapedQueryCompilingExpressionVisitorDependencies With([NotNull] ITypeMappingSource typeMappingSource) - => new ShapedQueryCompilingExpressionVisitorDependencies(EntityMaterializerSource, typeMappingSource); + => new ShapedQueryCompilingExpressionVisitorDependencies(EntityMaterializerSource, typeMappingSource, MemoryCache); + + /// + /// Clones this dependency parameter object with one service replaced. + /// + /// A replacement for the current dependency of this type. + /// A new parameter object with the given service replaced. + public ShapedQueryCompilingExpressionVisitorDependencies With([NotNull] IMemoryCache memoryCache) + => new ShapedQueryCompilingExpressionVisitorDependencies(EntityMaterializerSource, TypeMappingSource, memoryCache); } } diff --git a/test/EFCore.SqlServer.FunctionalTests/Query/QueryBugsTest.cs b/test/EFCore.SqlServer.FunctionalTests/Query/QueryBugsTest.cs index da7a38cf1d2..65c82d40952 100644 --- a/test/EFCore.SqlServer.FunctionalTests/Query/QueryBugsTest.cs +++ b/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() {