-
Notifications
You must be signed in to change notification settings - Fork 206
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
Move to using the new Roslyn IIncrementalGenerator API for better in-VS performance #1374
Merged
jkoritzinsky
merged 31 commits into
dotnet:feature/DllImportGenerator
from
jkoritzinsky:incremental-generator-tpi
Sep 8, 2021
Merged
Move to using the new Roslyn IIncrementalGenerator API for better in-VS performance #1374
jkoritzinsky
merged 31 commits into
dotnet:feature/DllImportGenerator
from
jkoritzinsky:incremental-generator-tpi
Sep 8, 2021
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…Info level instead of using symbols. This way, we can use the default comparer between TypePositionInfo instances to know if they're identical, even across compilations.
…e generator and write one test.
…te step so syntax generation only happens incrementally
…more incremental tests and stub out some that should theoretially work, but we can't actually test since GeneratorDriver doesn't support incrementality in the right way.
…more incremental tests and stub out some that should theoretially work, but we can't actually test since GeneratorDriver doesn't support incrementality in the right way.
…incremental generator.
…fferent, so no reason to keep around).
…elab into incremental-generator-tpi
50 tasks
…expressions during initial parsing.
@sharwell I can't seem to repro the failures locally. It looks like some sort of infra issue with the CodeAnalysis.Testing package. Can you take a quick look and see if you can figure out what's going on? |
sharwell
reviewed
Aug 3, 2021
AaronRobinsonMSFT
approved these changes
Aug 27, 2021
jkoritzinsky
added a commit
to jkoritzinsky/runtime
that referenced
this pull request
Sep 20, 2021
…VS performance (dotnet/runtimelab#1374) Commit migrated from dotnet/runtimelab@90c617d
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Through some local testing, it was determined that using the
ISourceGenerator
V1 API for DllImportGenerator will cause the same VS performance issues as dotnet/runtime#50741. Since the source our generator generates is required at design-time, we cannot take the same resolution as dotnet/runtime#50741 or use a simple non-incrementable direct port to the V2 API with the new "Implementation" source type to avoid impacting design time builds (dotnet/roslyn#54798).As a result, this PR moves our source generator to a more incremental design.
I've split the pipeline of the generator into a few sections:
GeneratedDllImport
attribute infoISymbol
-implementing types. Symbols are not equatable across compilations, so any stages where they are in the output are not cacheable between generator runs.As part of this refactoring, I moved
TypePositionInfo
to use a new type model to represent the managed type of the parameter/return value instead of symbols due to the caching issues mentioned above.The testing for the incremental support is an ad-hoc testing model that will be replaced when dotnet/roslyn#54832 is approved, implemented, and available.
I have validated with this branch that there is no typing lag when working in the System.Private.CoreLib solution from dotnet/runtime on a Surface Book 1 i7 (my work laptop).