[release/7.0-rc1] Don't track current field of state machines #74216
Conversation
This file contains hidden or 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
ghost
added
the
MichalStrehovsky
approved these changes
Aug 19, 2022
jeffschwMSFT
approved these changes
Aug 19, 2022
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
Backport of #74111 to release/7.0-rc1
/cc @MichalStrehovsky
Customer Impact
If customer code has many
yield returnstatements and the method also requires reflection dataflow analysis, the compiler would take a long time to analyze the method. This would look like a hang that is hard to troubleshoot (it can increase compilation times by tens of minutes). These hangs could be a support cost because they're not easy to root cause (no obvious crash with a printed stack trace that can be cheaply bucketed, etc.).Testing
We found this in one of the libraries tests. The test now compiles in a reasonable amount of time. The change also passes all NativeAOT testing and all IL Linker unit tests (this issue was in a shared codebase with IL Linker and we made the fix there first).
Risk
The issue is in analysis of compiler-generated state machine that is a new feature in .NET 7. We have unit tests for the core scenarios and they all pass.