Skip to content
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

Merge branch 'release/9.0-staging' into release/9.0 #35255

Merged
merged 17 commits into from
Dec 2, 2024

Conversation

AndriySvyryd
Copy link
Member

No description provided.

roji and others added 17 commits November 14, 2024 18:31
…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>
…g DbContext using IDesignTimeDbContextFactory (#35230)

Fixes #35174
@AndriySvyryd AndriySvyryd requested a review from a team December 2, 2024 21:31
@AndriySvyryd AndriySvyryd merged commit bb72406 into release/9.0 Dec 2, 2024
7 checks passed
@AndriySvyryd AndriySvyryd deleted the merge-release-9.0 branch December 2, 2024 22:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants