-
Notifications
You must be signed in to change notification settings - Fork 5.3k
Simplify ComInterfaceGenerator's Initialize() method #119360
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR optimizes the ComInterfaceGenerator's Initialize() method to reduce overhead for projects that don't use generated COM interfaces. The key change consolidates multiple incremental pipeline steps into fewer operations to improve initialization performance.
- Consolidates the multi-step incremental pipeline into fewer, more comprehensive steps
- Introduces ItemAndSyntaxes type to group interface contexts with their generated syntax nodes
- Removes unused utility methods from DiagnosticOr and IncrementalValuesProviderExtensions
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| IncrementalValuesProviderExtensions.cs | Removes unused Zip method and adds static keyword to lambda expressions |
| DiagnosticOr.cs | Removes unused utility methods (AddDiagnostic, WithValue, SplitArrays, Split, FilterAndReportDiagnostics overloads) |
| ItemAndSyntaxes.cs | New type to hold interface context with associated syntax nodes for simplified pipeline |
| ComInterfaceGenerator.cs | Major refactoring to consolidate pipeline steps and use ItemAndSyntaxes for grouping |
| ComClassGenerator.cs | Updates to use ItemAndSyntaxes pattern for consistency |
| RuntimeComApiUsageWithSourceGeneratedComAnalyzer.cs | Adds static keyword to lambda expression |
src/libraries/System.Runtime.InteropServices/gen/ComInterfaceGenerator/ItemAndSyntaxes.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Runtime.InteropServices/gen/ComInterfaceGenerator/ComInterfaceGenerator.cs
Outdated
Show resolved
Hide resolved
|
Tagging subscribers to this area: @dotnet/interop-contrib |
The ComInterfaceGenerator takes a significant amount of time in its Initialize() method setting up an elaborate pipeline, even for compilations that don't use generated COM interfaces. In particular, it spent a lot of time in the series of Zip that preceded writing the source out. This PR reduces the number of incremental steps the generator creates, reducing the Initialize() time for projects that don't need the generator. This will likely reduce incremental performance slightly, but the tradeoff seems worth it considering the vast majority of projects won't need this generator.
The change moves all steps in the pipeline preceding the creation of the IncrementalMethodStubGenerationContext to a single step. It also moves the generation of all syntaxes into a single step rather than creating them in parallel steps and Ziping them together. The ItemAndSyntaxes type is used to keep the InterfaceContext (which is required in the source-writing step) in the collection so it doesn't need to be re-zipped in before writing out.
After this change, there are a number of other simplifications we can make (we don't need so many intermediate steps within the first step, the DiagnosticOr concept can be simplified) that I'd like to add in follow-up PRs.
Addresses #119183