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

Don't track current field of state machines #2979

Merged
merged 2 commits into from
Aug 17, 2022

Conversation

sbomer
Copy link
Member

@sbomer sbomer commented Aug 17, 2022

Fixes the performance issue tested in #2977.
The problem was that we were tracking all possible returned values throughout the method, since we were treating them the same as hoisted locals. I'm sure it's still possible to come up with testcases that show bad performance (for example assigning many arrays to a hoisted local in the same iterator), but this should at least fix the immediate issue encountered in runtime.

Copy link
Member

@jtschuster jtschuster left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

@sbomer sbomer merged commit bc1f05f into dotnet:main Aug 17, 2022
@MichalStrehovsky
Copy link
Member

I tried this on my repro case and now the compilation finishes in reasonable time. Thanks!

MichalStrehovsky added a commit to MichalStrehovsky/runtime that referenced this pull request Aug 17, 2022
MichalStrehovsky added a commit to dotnet/runtime that referenced this pull request Aug 19, 2022
github-actions bot pushed a commit to dotnet/runtime that referenced this pull request Aug 19, 2022
jeffschwMSFT pushed a commit to dotnet/runtime that referenced this pull request Aug 20, 2022
Port of dotnet/linker#2979

Fixes #73048.

Co-authored-by: Michal Strehovský <MichalStrehovsky@users.noreply.github.com>
agocke pushed a commit to dotnet/runtime that referenced this pull request Nov 16, 2022
There was a performance problem where we were tracking all
possible returned values throughout the method, since we were
treating them the same as hoisted locals. I'm sure it's still
possible to come up with testcases that show bad performance (for
example assigning many arrays to a hoisted local in the same
iterator), but this should at least fix the immediate issue
encountered in runtime.

Commit migrated from dotnet/linker@bc1f05f
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.

3 participants