Fix to #35208 - Query/Perf: don't compile liftable constant resolvers in interpretation mode #35209
+4
−2
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
roji
approved these changes
Nov 26, 2024
There was a problem hiding this comment.
Am noting that the performance gap of running interpreted vs. compiled lambdas (as measured in the above benchmark) is indeed known and wasn't in question; the problem is what to do with lambdas which we know are only ever going to be evaluated once.
In #29814 (for 8.0) I changed some compiled delegates to interpreted (in the funcletizer), since the overhead of compiling/JIT is quite big, the lambdas to be evaluated are generally very small, and they're only ever going to get evaluated once in this context (see this benchmark). In other words, if these resolver lambdas really are only evaluated once (as part of query compilation), this change should actually make query compilation slower rather than faster (and not have an impact on runtime, since liftable constant resolvers should be part of runtime at all).
So I'd definitely want us to actually understand what's going on here... But since we're on a tight schedule, approving for now.
… in interpretation mode when the resolver itself contains a lambda In EF9 we changed the way we generate shapers in preparation for AOT scenarios. We no longer can embed arbitrary objects into the shaper, instead we need to provide a way to construct that object in code (using LiftableConstant mechanism), or simulate the functionality it used to provide. At the end of our processing, we find all liftable constants and for the non-AOT case we compile their resolver lambdas and invoke the result with liftable context object to produce the resulting constant object we initially wanted. (in AOT case we generate code from the resolver lambda). Problem is that we are compiling the resolver lambda in the interpretation mode - if the final product is itself a delegate, that delegate will itself be in the interpreter mode and therefore less efficient. Fix is to use regular compilation rather than interpretation. Fixes #35208 This is part of a fix for a larger perf issue: #35053
maumar
added a commit
that referenced
this pull request
Nov 26, 2024
Port of #35209 **Description** In EF9 we changed the way we generate shapers in preparation for AOT scenarios. We no longer can embed arbitrary objects into the shaper, instead we need to provide a way to construct that object in code (using LiftableConstant mechanism), or simulate the functionality it used to provide. At the end of our processing, we find all liftable constants and for the non-AOT case we compile their resolver lambdas and invoke the result with liftable context object to produce the resulting constant object we initially wanted. (in AOT case we generate code from the resolver lambda). Problem is that we are compiling the resolver lambda in the interpretation mode - if the final product is itself a delegate, that delegate will itself be in the interpreter mode and therefore less efficient when invoked multiple times when the query runs. Fix is to use regular compilation rather than interpretation. **Customer impact** Queries using collection navigation with significant amount of data suffer large performance degradation when compared with EF8. No good workaround. **How found** Multiple customer reports on 9.0.0 **Regression** Yes, from 8.0. **Testing** Ad-hoc perf testing with BenchmarkDotNet. Functional change already covered by numerous tests. **Risk** Low, quirk added.
maumar
changed the title
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
In EF9 we changed the way we generate shapers in preparation for AOT scenarios. We no longer can embed arbitrary objects into the shaper, instead we need to provide a way to construct that object in code (using LiftableConstant mechanism), or simulate the functionality it used to provide. At the end of our processing, we find all liftable constants and for the non-AOT case we compile their resolver lambdas and invoke the result with liftable context object to produce the resulting constant object we initially wanted. (in AOT case we generate code from the resolver lambda). Problem is that we are compiling the resolver lambda in the interpretation mode - if the final product is itself a delegate, that delegate will itself be in the interpreter mode and therefore less efficient.
Fix is to scan the resolver expression and look for nested lambdas inside - if we find some, compile the resolver in the regular mode instead.
Fixes #35208
This is part of a fix for a larger perf issue: #35053