-
Notifications
You must be signed in to change notification settings - Fork 3.2k
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
[automated] Merge branch 'release/9.0' => 'main' #35256
Merged
Merged
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
(cherry picked from commit a3c5677)
Microsoft.DotNet.Arcade.Sdk , Microsoft.DotNet.Build.Tasks.Templating , Microsoft.DotNet.Helix.Sdk From Version 9.0.0-beta.24562.13 -> To Version 9.0.0-beta.24572.2 Co-authored-by: dotnet-maestro[bot] <dotnet-maestro[bot]@users.noreply.github.com>
…s when constructing shaper with PopulateCollection (#35207) 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, or simulate the functionality it used to provide. One of the examples was use of ValueComparers in PopulateIncludeCollection. Now instead of passing list of ValueComparer objects to use (which we can't reliably generate in code), we pass the delegate which is used to compare two values: ``` (left, right) => left == null ? right == null : right != null && Invoke((v1, v2) => v1 == v2, (int)left, (int)right) ``` This incurs a performance hit on some scenarios with collections, but can be improved by simplifying the delegate we use. Instead of having nested lambdas and using Invoke, we can inline the body of the nested lambda directly into the outer lambda, like so: ``` (left, right) => left == null ? right == null : right != null && (int)left == (int)right ``` This one change yields significant improvement in the affected scenarios (reducing both time spent and allocations): ef 9 before the Invoke fix | Method | Async | Mean | Error | StdDev | Op/s | Gen0 | Gen1 | Allocated | |-------------------------- |------ |---------:|--------:|--------:|------:|-----------:|----------:|----------:| | PredicateMultipleIncludes | False | 322.6 ms | 0.97 ms | 0.86 ms | 3.099 | 13000.0000 | 6000.0000 | 79.48 MB | | PredicateMultipleIncludes | True | 344.9 ms | 6.79 ms | 6.67 ms | 2.899 | 14000.0000 | 7000.0000 | 87.72 MB | ef 9 after the invoke fix | Method | Async | Mean | Error | StdDev | Op/s | Gen0 | Gen1 | Allocated | |-------------------------- |------ |---------:|--------:|--------:|------:|-----------:|----------:|----------:| | PredicateMultipleIncludes | False | 242.8 ms | 2.39 ms | 2.12 ms | 4.119 | 8000.0000 | 5000.0000 | 51.69 MB | | PredicateMultipleIncludes | True | 263.4 ms | 2.21 ms | 2.06 ms | 3.797 | 10000.0000 | 9000.0000 | 59.93 MB |
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.
…n/Average (#35216) * Return null when the type is nullable for Cosmos Max/Min/Average (#35173) * Return null when the type is nullable for Cosmos Max/Min/Average Fixes #35094 This was a regression resulting from the major Cosmos query refactoring that happened in EF9. In EF8, the functions Min, Max, and Average would return null if the return type was nullable or was cast to a nullable when the collection is empty. In EF9, this started throwing, which is correct for non-nullable types, but a regression for nullable types. * Added notes * Added quirks * Fix tests.
…lateIncludeCollection rather than inline (#35217) Fixes #35212 Port of #35213 **Description** In EF9 we changed the way we generate shapers in preparation for AOT scenarios. As part of these changes we started inlining some delegates passed to PopulateIncludeCollection (as well as couple other methods), rather than compiling them. For scenarios with large number of entities we see significant perf degradation when these delegates are inlined, as opposed to compiled (like we used to do in EF8). This change reverts to EF8 behavior. **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. Perf regression only, no functional regression here. **Testing** Ad-hoc perf testing with BenchmarkDotNet. Functional change already covered by numerous tests. **Risk** Low - essentially reverting back to EF8 behavior, quirk added. **Benchmarks** before the fix (but already with invoke fix and no interpretation) | Method | Async | Mean | Error | StdDev | Op/s | Gen0 | Gen1 | Allocated | |------------ |------ |---------:|--------:|---------:|------:|-----------:|----------:|----------:| | MultiInclue | False | 455.1 ms | 8.94 ms | 10.29 ms | 2.197 | 11000.0000 | 6000.0000 | 67.92 MB | | MultiInclue | True | 435.4 ms | 1.77 ms | 1.66 ms | 2.297 | 11000.0000 | 6000.0000 | 67.92 MB | after the fix: | Method | Async | Mean | Error | StdDev | Op/s | Gen0 | Gen1 | Allocated | |------------ |------ |---------:|--------:|--------:|------:|----------:|----------:|----------:| | MultiInclue | False | 363.3 ms | 6.72 ms | 6.29 ms | 2.752 | 9000.0000 | 3000.0000 | 58.51 MB | | MultiInclue | True | 351.9 ms | 2.08 ms | 1.74 ms | 2.842 | 9000.0000 | 3000.0000 | 58.51 MB | This gets us pretty close to the EF8 numbers which were: | Method | Async | Mean | Error | StdDev | Op/s | Gen0 | Gen1 | Allocated | |------------ |------ |---------:|--------:|--------:|------:|----------:|----------:|----------:| | MultiInclue | False | 297.1 ms | 1.47 ms | 1.30 ms | 3.365 | 8000.0000 | 6000.0000 | 52.4 MB | | MultiInclue | True | 290.2 ms | 3.76 ms | 3.52 ms | 3.446 | 8500.0000 | 6000.0000 | 52.4 MB |
) (#35241) The transformation of equality/in-equality in a (negated) XOR is only possible when the expressions are BIT or integer types on the SQL side (i.e. taking value conversion into account). Similarly, the Boolean negation `NOT` can be implemented as `~` only if the underlying expression is a BIT. Fixes #35093. (cherry picked from commit e6abfdd) Co-authored-by: Andrea Canciani <ranma42@gmail.com>
github-actions
bot
requested review from
a team,
AndriySvyryd and
maumar
as code owners
December 2, 2024 22:39
AndriySvyryd
approved these changes
Dec 6, 2024
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.
I detected changes in the release/9.0 branch which have not been merged yet to main. I'm a robot and am configured to help you automatically keep main up to date, so I've opened this PR.
This PR merges commits made on release/9.0 by the following committers:
Instructions for merging from UI
This PR will not be auto-merged. When pull request checks pass, complete this PR by creating a merge commit, not a squash or rebase commit.
If this repo does not allow creating merge commits from the GitHub UI, use command line instructions.
Instructions for merging via command line
Run these commands to merge this pull request from the command line.
or if you are using SSH
After PR checks are complete push the branch
Instructions for resolving conflicts
Instructions for updating this pull request
Contributors to this repo have permission update this pull request by pushing to the branch 'merge/release/9.0-to-main'. This can be done to resolve conflicts or make other changes to this pull request before it is merged.
The provided examples assume that the remote is named 'origin'. If you have a different remote name, please replace 'origin' with the name of your remote.
or if you are using SSH
Contact .NET Core Engineering (dotnet/dnceng) if you have questions or issues.
Also, if this PR was generated incorrectly, help us fix it. See https://github.com/dotnet/arcade/blob/main/.github/workflows/scripts/inter-branch-merge.ps1.