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

Remove usage of CompilationProvider #8

Closed
wants to merge 4 commits into from

Conversation

dferretti
Copy link

Hey! A bit later than anticipated, but here is a draft PR showing some example code of how you could avoid using the CompilationProvider. I was moving things around between files and was not very organized and just dumped most everything into SymbolExtensions, so feel free to discard or refactor to your liking.

The main change: separate the path for when we are looking for types in external assemblies vs looking for types in the same C# project.

In the ParseMethod transform method, it now will optionally collect the matching types early on. The AttributeModel class now has a nullable array of ServiceRegistrationModels RegistrationsFromAssembly that can be set in cases where we collect these from external assemblies in this phase.

Separately, there is a new SyntaxProvider that hooks into any TypeDeclarationSyntax node. This will scan the current C# project for any type, and extract a few relevant bits from each type (name, IsGeneric, etc). This will create a lookup of all types that are declared in the project that should be cache friendly. The cache will have potentially lots of these objects, but it will still be smaller than the entire Compilation object.

This provider is combined with the method provider, and for attributes that don't specify an external assembly (or specify a type from the current assembly), the RegistrationsFromAssembly array will be null, and indicate that we should instead look through the source-code defined types.

I did have to slightly tweak the StringBuilder AppendLine, since that uses Environment.NewlLine internally. I am on Windows so that was causing my unit tests to fail locally, but that might also be due to my local autocrlf settings and you could ignore.

@Dreamescaper
Copy link
Owner

Thanks for the suggestion, I'll take a deeper look later!

Per my understanding, for the external assemblies case, assembly scanning will still happen for every keystroke, right? Only the cached data is potentially smaller?

@dferretti
Copy link
Author

Unfortunately yeah, I think based on this comment that yes, any transform method of a syntax provider will be re-run any time almost anything changes. I was banging my head against this for a while. I saw there was a context.MetadataReferencesProvider that I was hoping to use to get some better cached objects out of referenced assemblies, but I could not find anything useful there.

@Dreamescaper
Copy link
Owner

Dreamescaper commented Jun 20, 2024

I can see that even dotnet maintainers ignore that recommendation quite often:
https://github.com/search?q=CompilationProvider.Combine+OR+Combine%28context.CompilationProvider%29+OR+Combine%28compilation+org%3Adotnet&type=code

Not to mention third parties...

I'm going to wait for a while, maybe someone from Roslyn guys suggests anything useful: dotnet/roslyn#74001

I realized they've added an option to only run generators on saving or on builds in the latest VS Preview. Maybe they decided to "fix" this issue this way, not sure.

@dferretti
Copy link
Author

Yeah I don't think my changes here are all that helpful, it just replaces one problem for another. The assembly scanning in particular is a tough one since there is no good Syntax or other provider to get easy access to them.

I did have another thought overnight - not sure if it would be worth the effort but could be another option:

Collect the type info from public types in external assemblies in a separate pre-build step. Some BeforeTarget=CoreCompile msbuild task that extracts type info into a json file in the obj folder or something, and include it as an AdditionalFile.
Then you would have your 1 AdditionalTextsProvider that would not change very often, and 2 SyntaxProviders that do fire frequently but are only doing tiny amounts of work.

This issue seems like it could maybe help in the future, but I am not sure which direction they might take it. I could picture something like having a Syntax provider that is focused on doing this external assembly scanning, but can use the assembly MVID or something like that to be able to short circuit if nothing has changed

@dferretti
Copy link
Author

dferretti commented Jun 21, 2024

You might have already seen these, but here are some related issues that mention per-key updates and only needing to get info out of references, not syntax trees:

Edit: actually nvm - here is an example of Razor doing just that: https://github.com/dotnet/razor/blob/b4404959b514413320fa1f08ff418e1f600dc980/src/Compiler/Microsoft.CodeAnalysis.Razor.Compiler/src/SourceGenerators/RazorSourceGenerator.cs#L206-L215

@Dreamescaper
Copy link
Owner

Dreamescaper commented Jun 21, 2024

That's interresting.
So for external references - GetMetadataReference -> create empty compilation -> get types from it.
And this results would be cacheable.
Sounds interresting.

@dferretti
Copy link
Author

Closing this in favor of #9

@dferretti dferretti closed this Jun 21, 2024
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.

2 participants