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.

Sign up for GitHub

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

Jump to bottom

resolution time has *quadrupled* between 235d77457 and 6fa61b810 #33061

Closed
arielb1 opened this issue Apr 17, 2016 · 5 comments · Fixed by #33064
Closed

resolution time has *quadrupled* between 235d77457 and 6fa61b810 #33061

arielb1 opened this issue Apr 17, 2016 · 5 comments · Fixed by #33064
Labels
I-compiletime Issue: Problems and improvements with respect to compile times.

Comments

@arielb1
Copy link
Contributor

arielb1 commented Apr 17, 2016 •
edited
Loading

Before (on librustc):

time: 0.013; rss: 169MB language item collection
time: 0.281; rss: 201MB resolution

After (on same librustc):

time: 0.017; rss: 226MB language item collection
time: 1.055; rss: 277MB resolution

cc @jseyfried
@arielb1 arielb1 added the I-compiletime Issue: Problems and improvements with respect to compile times. label Apr 17, 2016
@arielb1
Copy link
Contributor Author

arielb1 commented Apr 17, 2016

The extra RAM usage seems to be because of the AST not getting freed after the early lint checks. Not sure if this is intentional.

@jonas-schievink
Copy link
Contributor

The AST should definitely still get freed, nobody touched the code doing that, so it's not intentional (unless something about the command line options has changed?)

@jseyfried
Copy link
Contributor

jseyfried commented Apr 17, 2016 •
edited
Loading

The performance regression in resolve looks like it was caused by #32328. I'll work on a fix now.
I'm not sure what caused the RAM increase.

@retep998
Copy link
Member

Resolution used to take about 1 seconds a few months ago in winapi and now it takes 9 seconds.

@jseyfried jseyfried mentioned this issue Apr 18, 2016
Merged
@jseyfried
Copy link
Contributor

I fixed the performance regression in #33064.

bors added a commit that referenced this issue Apr 18, 2016
@bors
Auto merge of #33064 - jseyfried:improve_resolve_performance, r=eddyb
63760ac 
resolve: Improve performance

This fixes #33061 by speeding up searchs through all traits in scope, a bottleneck in `resolve`.

According to my measurements, this PR improves resolution performance by a factor of 3.4x on `librustc`, which almost completely reverses the 3.75x performance regression reported in #33061.

r? @eddyb
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
I-compiletime Issue: Problems and improvements with respect to compile times.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

resolve: Improve performance jseyfried/rust
4 participants
@retep998 @jonas-schievink @arielb1 @jseyfried