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

Major async/await compiler performance regression #67706

Closed
Mark-Simulacrum opened this issue Dec 29, 2019 · 9 comments
Closed

Major async/await compiler performance regression #67706

Mark-Simulacrum opened this issue Dec 29, 2019 · 9 comments
Labels
A-async-await AsyncAwait-Triaged I-compiletime P-high T-compiler

Comments

@Mark-Simulacrum
Copy link
Member

@Mark-Simulacrum Mark-Simulacrum commented Dec 29, 2019

Something in #67670 caused a major performance regression. Query that regressed is normalize_ty_after_erasing_regions, but not sure if that means much. I suspect that #65244 is the PR responsible, but have no proof.

@Mark-Simulacrum Mark-Simulacrum added I-nominated I-compiletime T-compiler A-async-await labels Dec 29, 2019
@Centril
Copy link
Contributor

@Centril Centril commented Dec 29, 2019

Seems like the most prudent course of action is to create a revert PR for #65244 to see if perf is regained.

@Mark-Simulacrum
Copy link
Member Author

@Mark-Simulacrum Mark-Simulacrum commented Dec 29, 2019

I wouldn't be opposed to such a course. Not sure if I'll have the time to do so for a day or two, though.

bors added a commit that referenced this issue Jan 1, 2020
[perf] DO NOT MERGE test reverting #65244

This reverts commit f35517e.

Revert #65244 so we can see if it is the cause of the performance issue in #67706
@wesleywiser
Copy link
Member

@wesleywiser wesleywiser commented Jan 1, 2020

Testing a revert in #67768

@wesleywiser
Copy link
Member

@wesleywiser wesleywiser commented Jan 1, 2020

Looks like #65244 is the cause.

@pnkfelix
Copy link
Member

@pnkfelix pnkfelix commented Jan 2, 2020

triage: P-high; leaving nominated for discussion at meeting (namely discussion of revert)

@pnkfelix pnkfelix added the P-high label Jan 2, 2020
@pnkfelix
Copy link
Member

@pnkfelix pnkfelix commented Jan 2, 2020

Discussed in T-compiler meeting.

Members present generally approved a revert of PR #65244 to resolve this issue in short term.

@wesleywiser
Copy link
Member

@wesleywiser wesleywiser commented Jan 2, 2020

I guess I should re-open my revert then?

@Mark-Simulacrum
Copy link
Member Author

@Mark-Simulacrum Mark-Simulacrum commented Jan 2, 2020

Yes, please do, and I (or someone else) can review it, I think, fairly quickly.

bors added a commit that referenced this issue Jan 3, 2020
…crum

Revert #65244 for performance reasons

This reverts commit f35517e.

Revert #65244 so we can see if it is the cause of the performance issue in #67706

cc #67644
@tmandry
Copy link
Contributor

@tmandry tmandry commented Jan 7, 2020

Fixed in #67768.

@tmandry tmandry closed this Jan 7, 2020
@tmandry tmandry added the AsyncAwait-Triaged label Jan 7, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-async-await AsyncAwait-Triaged I-compiletime P-high T-compiler
Projects
None yet
Development

No branches or pull requests

5 participants