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

A lot of time spent in GVMDependenciesNode.SearchDynamicDependencies #452

Closed
jkotas opened this issue Dec 12, 2020 · 10 comments
Closed

A lot of time spent in GVMDependenciesNode.SearchDynamicDependencies #452

jkotas opened this issue Dec 12, 2020 · 10 comments
Labels
area-NativeAOT-coreclr .NET runtime optimized for ahead of time compilation

Comments

@jkotas
Copy link
Member

jkotas commented Dec 12, 2020

Repro:

  1. Clone https://github.com/jkotas/AvaloniaCoreRTDemo/tree/update-to-0.10.0-rc1
  2. dotnet publish -c Release -r win-x64

Result: The native compilation takes many time longer compared to previous Avalonia version (close to 10 minutes on my notebook)

The problem seems to be a lot of time spent under SearchDynamicDependencies:

Name Inc %
ILCompiler.Compiler!GVMDependenciesNode.SearchDynamicDependencies 74.0
- ILCompiler.TypeSystem!MetadataVirtualMethodAlgorithm.ResolveInterfaceMethodToVirtualMethodOnType 52.7
- ILCompiler.TypeSystem!MetadataVirtualMethodAlgorithm.FindVirtualFunctionTargetMethodOnObjectType 20.5
@jkotas jkotas added the area-NativeAOT-coreclr .NET runtime optimized for ahead of time compilation label Dec 12, 2020
@jkotas
Copy link
Member Author

jkotas commented Dec 12, 2020

@FoggyFinder Have you noticed much longer compilation times with 10.0-rc1 in your project as well?

@jkotas
Copy link
Member Author

jkotas commented Dec 12, 2020

@MichalStrehovsky Do you believe that we need a new cache to fix this, or should the existing caches be enough?

@FoggyFinder
Copy link

@FoggyFinder Have you noticed much longer compilation times with 10.0-rc1 in your project as well?

Yes. But that was expected. I saw somewhere a message that something heavy was added to 0.10-x version of Avalonia. I don't remember details though.

@MichalStrehovsky
Copy link
Member

We don't cache the results of virtual method resolution at all and the algorithm we have is pretty slow.

There have been several attempts to fix GVM analysis, but generic virtual methods are just too annoying - this is the one thing I envy C++ - no templated virtual methods there.

It's possible this is just hitting some bad pattern. I don't know much about this part of the codebase. David and Fadi were working in this area. I'll have to ramp up on this for default interface methods at some point.

@MichalStrehovsky
Copy link
Member

AvaloniaUI/Avalonia#3255 is the source of bad compilation throughput in more recent Avalonia. It introduced a number of generic interface methods on interfaces that are fundamental to Avalonia and implemented by dozens of classes, some of them even generic. We are spending most of the time figuring out what are all the possible method bodies and data structures we need to generate.

Every time a method like IAvaloniaObject.GetValue<XXX> is called with a different generic argument, we need to go over all types (also generic instantiated types) that implement IAvaloniaObject, find the implementation and ensure we'll have the implementation for the XXX instantiation argument on that implementation.

The IValueSink interface has the same problem (implemented by every class under the sun and the generic methods on the interface are called with countless number of generic arguments).

@MichalStrehovsky
Copy link
Member

Fixed in #493.

@grokys
Copy link

grokys commented Dec 28, 2020

Some background to this: in Avalonia we started with an untyped value store as in WPF, but customers were complaining about excessive boxing (which is also a criticism leveled at WPF). Because our DependencyProperty equivalents are typed, we were able to move to a value store which doesn't box by making use of generics. In future I was hoping to extend this to have a completely typed binding pipeline as well as value store - which would eliminate boxing entirely.

Is this a bad idea in terms of CoreRT? Or is it simply something that the CoreRT compiler is catching up on?

@jkotas
Copy link
Member Author

jkotas commented Dec 28, 2020

This design is increasing the static footprint of Avalonia applications. The increased static footprint results in longer startup times with JIT. It results into larger binary sizes and longer compile times with AOT.

In other words, you are trading-off one performance problem (boxing) for a different performance problem (startup time or larger binary sizes).

I see data for several micro-benchmarks in your PR. Micro-benchmarks do not tell the full story for this type of changes. To understand the impact of this design change, you would need to measure startup time (with JITed runtime) and observable improvement due to reduced boxing in a large Avalonia application.

@grokys
Copy link

grokys commented Dec 28, 2020

Hmm, interesting. Looks like we may need to reconsider our plans. Is there a good way to measure JIT overhead of .NET applications? AFAIK Benchmark.Net goes out of its way to not include this in its results.

@jkotas
Copy link
Member Author

jkotas commented Dec 28, 2020

The usual way to measure startup time is to measure time from process start to the point where the application is started up and responsive. It can be done either by the application self-measuring itself; or via events.

https://github.com/dotnet/performance repo has some tooling for this, but it may be hard to reuse.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-NativeAOT-coreclr .NET runtime optimized for ahead of time compilation
Projects
None yet
Development

No branches or pull requests

4 participants