Skip to content

Speed up operations on scopes #27943

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

Merged
merged 7 commits into from
Aug 25, 2015
Merged

Speed up operations on scopes #27943

merged 7 commits into from
Aug 25, 2015

Conversation

arielb1
Copy link
Contributor

@arielb1 arielb1 commented Aug 22, 2015

This increases regionck performance greatly - type-checking on
librustc decreased from 9.1s to 8.1s. Because of Amdahl's law,
total performance is improved only by about 1.5% (LLVM wizards,
this is your opportunity to shine!).

before:
576.91user 4.26system 7:42.36elapsed 125%CPU (0avgtext+0avgdata 1142192maxresident)k
after:
566.50user 4.84system 7:36.84elapsed 125%CPU (0avgtext+0avgdata 1124304maxresident)k

I am somewhat worried really need to find out why we have this Red Queen's
Race going on here. Originally I suspected it may be a problem from RFC1214's
warnings, but it seems to be an effect from other changes.

However, the increase seems to be mostly in LLVM's time, so I guess
it's the LLVM wizards' problem.

r? @nikomatsakis

@nikomatsakis
Copy link
Contributor

Nice. I was thinking of starting a branch to follow up in these ideas, glad I didn't. Will review Monday probably (if not sooner)/

@arielb1
Copy link
Contributor Author

arielb1 commented Aug 22, 2015

I will give perf numbers for the last patch when my benchmark machine is working again.

@nikomatsakis
Copy link
Contributor

(started a review, mostly jutts nits so far, going to have to finish up tomorrow.)

@bors
Copy link
Collaborator

bors commented Aug 24, 2015

☔ The latest upstream changes (presumably #27856) made this pull request unmergeable. Please resolve the merge conflicts.

Ariel Ben-Yehuda and others added 5 commits August 24, 2015 20:10
this should reduce the size of ty::Region to 24 bytes (from 32),
and they are treated differently in most cases anyway.
This reduces the size of CodeExtent to 12 bytes (was 24). We should have
a warning for this kind of problem.
This increases regionck performance greatly - type-checking on
librustc decreased from 9.1s to 8.1s. Because of Amdahl's law,
total performance is improved only by about 1.5% (LLVM wizards,
this is your opportunity to shine!).

before:
576.91user 4.26system 7:42.36elapsed 125%CPU (0avgtext+0avgdata 1142192maxresident)k
after:
566.50user 4.84system 7:36.84elapsed 125%CPU (0avgtext+0avgdata 1124304maxresident)k

I am somewhat worried really need to find out why we have this Red Queen's
Race going on here. Originally I suspected it may be a problem from RFC1214's
warnings, but it seems to be an effect from other changes.

However, the increase seems to be mostly in LLVM's time, so I guess
it's the LLVM wizards' problem.
this makes the code cleaner
@nikomatsakis
Copy link
Contributor

@bors r+ -- looks nice.

cc @pnkfelix just fyi, this touches some of the setup in region.rs, though I'd make you aware

@bors
Copy link
Collaborator

bors commented Aug 24, 2015

📌 Commit 1e507d4 has been approved by nikomatsakis

@bors
Copy link
Collaborator

bors commented Aug 24, 2015

⌛ Testing commit 1e507d4 with merge c1b445e...

@arielb1
Copy link
Contributor Author

arielb1 commented Aug 24, 2015

@bors r=nikomatsakis 06563fe

@bors
Copy link
Collaborator

bors commented Aug 24, 2015

⌛ Testing commit 06563fe with merge bc35734...

bors added a commit that referenced this pull request Aug 24, 2015
This increases regionck performance greatly - type-checking on
librustc decreased from 9.1s to 8.1s. Because of Amdahl's law,
total performance is improved only by about 1.5% (LLVM wizards,
this is your opportunity to shine!).

before:
576.91user 4.26system 7:42.36elapsed 125%CPU (0avgtext+0avgdata 1142192maxresident)k
after:
566.50user 4.84system 7:36.84elapsed 125%CPU (0avgtext+0avgdata 1124304maxresident)k

I am somewhat worried really need to find out why we have this Red Queen's
Race going on here. Originally I suspected it may be a problem from RFC1214's
warnings, but it seems to be an effect from other changes.

However, the increase seems to be mostly in LLVM's time, so I guess
it's the LLVM wizards' problem.

r? @nikomatsakis
@bors bors merged commit 06563fe into rust-lang:master Aug 25, 2015
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