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

Infinite generic expansions cause efcore perform poorly with AOT compilation #20393

Closed
hez2010 opened this issue Mar 24, 2020 · 7 comments · Fixed by #24211
Closed

Infinite generic expansions cause efcore perform poorly with AOT compilation #20393

hez2010 opened this issue Mar 24, 2020 · 7 comments · Fixed by #24211
Labels
area-perf closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. community-contribution customer-reported type-enhancement
Milestone

Comments

@hez2010
Copy link
Contributor

hez2010 commented Mar 24, 2020

Infinite generic expansions cause efcore perform poorly with AOT compilation

Original issue in CoreRT see dotnet/corert#8052

Both ExecuteImplementation and ExecuteImplementationAsync call themselves with different generic arguments and lead to infinite generic expansions, which make efcore impossible to be AOT compiled.

Source:
https://github.com/dotnet/efcore/blob/master/src/EFCore/Storage/ExecutionStrategy.cs#L183
and
https://github.com/dotnet/efcore/blob/master/src/EFCore/Storage/ExecutionStrategy.cs#L276

Above recursive calls should be removed.

@hez2010 hez2010 changed the title Infinite generic expansions cause efcore hard to be AOT compiled Infinite generic expansions cause efcore impossible to be AOT compiled Mar 24, 2020
@ajcvickers
Copy link
Member

This is perfectly valid C# code. Per discussion with .NET director, the AOT implementation needs to handle things like this.

@jkotas
Copy link
Member

jkotas commented Mar 24, 2020

@ajcvickers You are right that this is a valid code. It is just a code with a very poor performance characteristics. This code is going "work" with CoreCLR, but it will cause the CoreCLR native images to be much larger than necessary.

When we find similar patterns in our frameworks, we generally fixed them. For example, System.Reflection.Metadata had a pattern that we have fixed. Other popular packages in ecosystem are happy to fix these performance problems as well. For example, System.Reactive fixed one of these infinite generic recursions as well.

If you are happy with EFCode having poor performance, on .NET Core w/ CoreCLR, feel free to close this again.

@jkotas jkotas reopened this Mar 24, 2020
@jkotas jkotas changed the title Infinite generic expansions cause efcore impossible to be AOT compiled Infinite generic expansions cause efcore perform poorly with AOT compilation Mar 24, 2020
@ajcvickers
Copy link
Member

@jkotas If there are performance issues. then we will address those appropriately by profiling, etc. in the normal way. Still, AOT needs to work with this code.

@jkotas
Copy link
Member

jkotas commented Mar 24, 2020

those appropriately by profiling, etc

Make sure to look at binary sizes and startup times after AOT compilation, for the different AOT flavors that we are shipping in .NET.

@hez2010
Copy link
Contributor Author

hez2010 commented Oct 14, 2020

I'm afriad that without Universal Shared Generics, infinite recursive generics may be impossible to be compiled with a Native AOT compiler. It only works when there's a JIT at runtime. Therefore, to support Native AOT compilation, it's necessary to rewrite the code and avoid infinite recursive generics.

@roji
Copy link
Member

roji commented Oct 14, 2020

@hez2010 EF Core 6.0 will likely have a focus area into perf and AOT (e.g. linking), so I'm hoping we'll take a full look at this.

@darkguy2008
Copy link

@roji thanks, really looking forward to this. It's causing a problem when building any project that includes EFCore in its dependencies, which is one of the most widely used libraries for .NET out there. Definitely worth the effort. I've set up a repo there ( dotnet/runtimelab#283 ) where you can replicate the issue if needed. I'll leave the repo alive until then. Thanks!

@AndriySvyryd AndriySvyryd removed their assignment Feb 22, 2021
@AndriySvyryd AndriySvyryd added the closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. label Feb 22, 2021
@ajcvickers ajcvickers modified the milestones: 6.0.0, 6.0.0-preview2 Mar 5, 2021
@ajcvickers ajcvickers modified the milestones: 6.0.0-preview2, 6.0.0 Nov 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-perf closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. community-contribution customer-reported type-enhancement
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants