YairHalberstadt / stronginject

compile time dependency injection for .NET
MIT License
845 stars 24 forks source link

Convert to IIncrementalGenerator #179

Closed sharwell closed 2 years ago

sharwell commented 2 years ago

The current use of SG v1 API results in excessive overhead for users working with Visual Studio 2022 and newer.

The central problem in the current approach is the iteration over all syntax trees with each step:

https://github.com/YairHalberstadt/stronginject/blob/84ac7809880f683956c6ffcf4e3752cfd289f954/StrongInject.Generator/SourceGenerator.cs#L32

This makes the source generator O(project size) for every edit.

jnm2 commented 2 years ago

In the largest project where we use StrongInject, this is causing the light bulb to take 5-20 seconds to appear and 5-20 seconds to preview or apply any suggested action. (Or invoke renames, etc.)

sharwell commented 2 years ago

I'm investigating the work required to make this change.

YairHalberstadt commented 2 years ago

If it's mainly the syntax trees that are expensive, then it should be relatively straightforward to use an incremental generator for that step.

YairHalberstadt commented 2 years ago

@jnm2 here's my first pass at creating an incremental generator which should reduce the amount of time iterating through syntax trees:

https://github.com/YairHalberstadt/stronginject/tree/incremental-generators

How annoying would it be to ask you to build and reference from source, and tell me if it reduces light bulb wait time?

Thanks!

jnm2 commented 2 years ago

Easy. I checked out that branch (24057fd60fc168c76984a154b231974d4340e2cf), added StrongInject.csproj and StrongInject.Generator.csproj to the solution in question, and replaced the StrongInject package reference with a project reference to StrongInject.csproj and added an additional project reference with OutputItemType="Analyzer" to StrongInject.Generator.csproj to the project that needs source generation. The solution builds fine.

I then restarted VS, built again (incremental build succeeded immediately), and invoked the light bulb on random things. It seems like the light bulb takes the same amount of time to open after any source edit, and it seems like the fixes take 2-3 times longer to preview or apply than they did before. Very unscientific perceptual guess.

I'm not sure what my testing methodology should be. Sam said:

the problem here was indirect. extreme allocations from StrongInject are leading to long GC pauses. So far attributed 37GB allocs to it in your trace

So if the light bulb and fixes started applying very fast, I'd potentially still have to spend some time doing real work in the solution in order to fully evaluate the improvement?

I also checked out the commit in this project just prior to introducing StrongInject, and the light bulb and fixes became perceptually instant. That gives me something to perceptually compare to. Checking out the commit that introduced StrongInject brings back the long waits.

YairHalberstadt commented 2 years ago

Well that's disappointing...

Can I get an idea for how big your project is, say in number of lines of code and number of files? And do you know if you get similar slowness in CLI build times, or when working in VSCode or rider? I want to make sure that I can reproduce this without having access to Visual Studio.

jnm2 commented 2 years ago

2510 files, 280k lines including blanks. I couldn't see a significant effect in CLI build times. (I got anywhere from 1-3 seconds additional CLI build time when I initially added StrongInject, but my recent comparisons fluctuated enough that it might be closer to 0 seconds.)

Rider's light bulb is always instant and the fixes vary from instant to less than a second to apply, for comparable operations.

YairHalberstadt commented 2 years ago

Ok, that's a reasonable size for a single project. As a temporary workaround have you considered separating the StrongInject code to a different project?

jnm2 commented 2 years ago

By introducing a new entry point assembly that defines the container? (All application-level code where the light bulb needs to work is in classes injected by StrongInject.)

Every project in the solution references StrongInject, also, because Owned<T> is sprinkled throughout. Does that incur a cost, or is the cost delayed until an IContainer<T> implementation is seen?

YairHalberstadt commented 2 years ago

I imagine everything will incur the cost because there's no way to reference StrongInject without the generator.

YairHalberstadt commented 2 years ago

@sharwell I've created an incremental generator which I assume should avoid scanning all trees on every edit, but according to @jnm2 it's not improving performance. Would you be able to advise if it does what it says in the tin?

The generator is at https://github.com/YairHalberstadt/stronginject/tree/incremental-generators

jnm2 commented 2 years ago

I wonder if ExcludeAssets="analyzers" affects generators.

sharwell commented 2 years ago

Busy today but will try to take a look later

sharwell commented 2 years ago
YairHalberstadt commented 2 years ago

@sharwell @jnm2 Is there some way you can share the full trace with me? Also what's the easiest way to visualise the trace?

Thanks!

jnm2 commented 2 years ago

(I shared dotMemory and dotTrace traces of devenv.exe while invoking the light bulb directly with Yair, and it matches closely with Sam's screenshot above)

sharwell commented 2 years ago

I submitted #182 to fix TryResolve and ResolveDecorators.

YairHalberstadt commented 2 years ago

Fixed by #180