QueryCache misses for dynamically created queries with ConstantExpresssions or nested MemberExpressions from closure #8909
Comments
|
It seems that is the usage of
|
|
Is it the same when using example For the contains it generate an SQL with the values in the statement, not as parameters. Will the cache handle that to replace the parameters? |
|
@Tasteful Sorry you are hitting this. Can you post the complete LINQ query that causes the cache miss? |
|
@anpete I will try to make a breakout of relevant code parts. |
Any reason you haven't upgraded to 1.1.2? |
|
Oh yeah, definitely try that 😄 |
|
Was it the one that i found with Db.Set, then I already have that fix localy. Was it any other fixes in 1.1.2 regarding to query-cache? It was found in production system and want to be able to produce same issue localy on the same version before upgrade and trying any fixes. |
|
@anpete Tested this with both the 1.1.1 and 1.1.2 release and both version have the same behavour I have put a test-repo here https://github.com/Tasteful/bugs/blob/query-cache2 and if you check the https://github.com/Tasteful/bugs/blob/query-cache2/MemoryUsage/Program.cs and specific the method and compare that to If I not can add and use the Then we have method For all versions that not is working I found that we use the I will continue with the larger SQL that was duplicated 16 000 times and see if I can make a simple replication there also. |
|
If I remember correct the QueryCache is caching all the queries infinity, is that a big benefit to do that instead of have a sliding cache timeout to clear out queries that not is used frequently? |
|
The first thing is that there should only be one entry for each query, so we will look at that (thanks for the repro). We are using the ASP.NET MemoryCache, which does have some configuration options. |
|
What I can see in https://github.com/aspnet/EntityFramework/blob/dev/src/EFCore/Query/Internal/CompiledQueryCache.cs is that no options are set when the item is adding to the cache. The old v1 implementation of that is removing 10% of cache entries when gc gen 2 is happening. This will be changed in v2 aspnet/Announcements#228 and if no expiration time is set the cache entries will probably live forever. |
|
@Tasteful I guess we could set a value for SlidingExpiration as a mitigation for these kinds of bugs. I think you could also manually call the Compact method as a workaround. |
|
For the one where the same SQL is stored multiple times the callstack is the following and result down into a stringbuilder. The string builder internal use an char arrayto keep track of the data. I'm not sure if this string builder is populated with data for each query or that the
The expression for that looks like and I can see that The inner part is created from an method as below and added to the query with |
|
@Tasteful I have made some progress on this. It is indeed to do with constant nodes in the expression tree. Normally when we see a constant node in the tree, it is because the value was in-lined in the query directly. E.g. a c# literal. Hence, we make the decision to not parameterize it because it is essentially hard-coded in the query. On the other hand, values coming from variables are captured by the compiler into a closure, and show up in the expression tree as member access expressions. These we parameterize. So, to start with, I think a workaround for this case is to introduce member access nodes in your dynamic LINQ expression instead of constants (by assigning the list of values to a field on a holder object and then creating a member expression to the field). |
|
@anpete Thanks for the proposed workaround, I will try to rewrite the code to user member access expressions instead. This is "little tricky" to find as an developer that only is using the library. Are you consider to rewrite the internal part to also parameterize the constant expression node or does it exists any other smart thing to make better decission for real constants vs. local variables? |
|
Things we can do:
Punting for now. |
|
@anpete Before adding call to |
|
@anpete Thanks. I was running into this issue as well, using constants when building expressions from our internal query objects. Changing to member access made the process memory usage flat and improved perf quite a bit too. |
|
Great to hear the workaround is somewhat acceptable. 😄 |
|
@anpete I have some problems to get it to work. Not sure what I'm doing wrong, maybe you can point me in correct direction. I put a short example here https://github.com/Tasteful/bugs/blob/query-cache-example/MemoryUsage/Program.cs with 3 different methods to create the expression and only the one with The hash and comparison is done with the Output from program, original code was like the one in method Example CreateExpression
hash1: -1699865404
hash2: -1699865404
eq: True
Example CreateExpression2
hash1: -756323298
hash2: 321098734
eq: False
Example CreateExpression3
hash1: -561453146
hash2: 1704667454
eq: False |
|
@Tasteful Are you running parameter extraction over the ET first? I.e. Use ParameterExtractingExpressionVisitor to turn the member expressions into parameters. |
|
@anpete No, I didn't. I was extracting the expressions that I run inside the DbContext-object and was thinking that the object was "ready to calculate hash on" I will rewrite and execute them in the DbContext to get the ParameterExtractingExpressionVisitor to run. I will come back with result. |
|
@anpete When I run the code as a query in the DbContext it was working. Then I need to rewrite the unit test to also use the ParameterExtractingExpressionVisitor to test the specific methods that is creating the method. Thanks for the help. |
|
@anpete I still have problem with rewriting the expression into usage of Code that is working but I'm not able to use due that I need to construct the expressions dynamic, (headline in output var id = Guid.NewGuid().ToString("N");
var items = dbContext.MyTable1
.Where(item => dbContext.MyTable2
.Where(x => x.Id == item.Id)
.Where(x => x.Name == id)
.Any())
.ToList();Manually creating lambda expression that is used where-expression. For me this should be equalent to the above code. (headline in output var id = Guid.NewGuid().ToString("N");
Expression<Func<MyTable2, bool>> whereExpression = item => item.Name == id;
var items = dbContext.Set<MyTable1>()
.Where(item => dbContext.Set<MyTable2>()
.Where(x => x.Id == item.Id)
.Where(whereExpression)
.Any())
.ToList();Manually creating expression that is used where-expression. (headline in output var id = Guid.NewGuid().ToString("N");
Expression<Func<string>> valueExpression = () => id;
var param = Expression.Parameter(typeof(MyTable2), "item");
var property = Expression.Property(param, nameof(MyTable2.Name));
Expression result = Expression.Equal(property, Expression.Invoke(valueExpression));
var whereExpression = Expression.Lambda<Func<MyTable2, bool>>(result, param);
var items = dbContext.Set<MyTable1>()
.Where(item => dbContext.Set<MyTable2>()
.Where(x => x.Id == item.Id)
.Where(whereExpression)
.Any())
.ToList();Program output Query cache is working, using the dbContext.MyTable2
Before iteration 0 query cache count 0
After iteration 0 query cache count 1
Before iteration 1 query cache count 1
After iteration 1 query cache count 1
Before iteration 2 query cache count 1
After iteration 2 query cache count 1
Before iteration 3 query cache count 1
After iteration 3 query cache count 1
Before iteration 4 query cache count 1
After iteration 4 query cache count 1
Query cache is not working, using the dbContext.Set<MyTable2>().Where(LambdaExpression)
Before iteration 0 query cache count 1
After iteration 0 query cache count 2
Before iteration 1 query cache count 2
After iteration 1 query cache count 3
Before iteration 2 query cache count 3
After iteration 2 query cache count 4
Before iteration 3 query cache count 4
After iteration 3 query cache count 5
Before iteration 4 query cache count 5
After iteration 4 query cache count 6
Query cache is not working, using the dbContext.Set<MyTable2>().Where(manual created expression)
Before iteration 0 query cache count 6
After iteration 0 query cache count 7
Before iteration 1 query cache count 7
After iteration 1 query cache count 8
Before iteration 2 query cache count 8
After iteration 2 query cache count 9
Before iteration 3 query cache count 9
After iteration 3 query cache count 10
Before iteration 4 query cache count 10
After iteration 4 query cache count 11 |
|
It seems that this is a regression bug from my earlier PR #4431 To generate correct hashcode for multiple queries the change below need to be changes (PR is on the way) And for the older 1.1.2 release |
|
From what I am hearing there seem to be actions to take in 2.0. Clearing up milestone so that this doesn’t get lost in the backlog. |
|
@smitpatel What's the status on this one? Do we know what needs to happen to trigger a memory leak? |
|
There are 2 issues here, For the second issue, |
smitpatel
added
the
closed-fixed
|
Filed #9105 for patch consideration. |
smitpatel
changed the title
… ConstantExpresssions or nested MemberExpressions from closure Adding parametrization for nested lambda expressions
… ConstantExpresssions or nested MemberExpressions from closure Adding parametrization for nested lambda expressions
instead of constant member access expression must be generated to avoid cache issues. see: dotnet/efcore#8909 see: dotnet/efcore#10535
I got a lot of dupplicated entries in the
MemoryCachethat origin from EF and many of them contains the same sql query and is generated from the exact same code.Example the following that generate an SQL query that is duplicated in cache more than 16 000 times.
When I checking the entity in the
MemoryCachewe can se a lot of repeated entries that is 14 338 bytes each but contains the same query.The only "special" that I can remember that I have done is that the query is built with
System.Linq.Expressions.Expressionthat is added to the query with.Where(expression).What is the best approach to find the reason for this cache misses?
Further technical details
EF Core version: 1.1.1
Database Provider: Microsoft.EntityFrameworkCore.SqlServer
Operating system: Windows 10
IDE: Visual Studio 2017
The text was updated successfully, but these errors were encountered: