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

encode region::Scope using fewer bytes #44809

Merged
merged 5 commits into from
Sep 25, 2017
Merged

Conversation

arielb1
Copy link
Contributor

@arielb1 arielb1 commented Sep 24, 2017

Now that region::Scope is no longer interned, its size is more important. This PR encodes region::Scope in 8 bytes instead of 12, which should speed up region inference somewhat (perf testing needed) and should improve the margins on #36799 by 64MB (that's not a lot, I did this PR mostly to speed up region inference).

This is a perf-sensitive PR. Please don't roll me up.

r? @eddyb

This is based on #44743 so I could get more accurate measurements on #36799.

This wraps unsizing coercions within an additional level of
`commit_if_ok`, which rolls back type variables if the unsizing coercion
fails. This prevents a large amount of type-variables from accumulating
while type-checking a large function, e.g. shaving 2GB off one of the
4GB peaks in rust-lang#36799.
this should produce more error stability
Now that region::Scope is no longer interned, its size is more
important. This PR encodes region::Scope in 8 bytes instead of 12, which
should speed up region inference somewhat (perf testing needed) and
should improve the margins on rust-lang#36799 by 64MB (that's not a lot, I did
this PR mostly to speed up region inference).
@eddyb
Copy link
Member

eddyb commented Sep 24, 2017

Nice trick! @bors r+

@bors
Copy link
Contributor

bors commented Sep 24, 2017

📌 Commit c10b23e has been approved by eddyb

@Mark-Simulacrum
Copy link
Member

@bors p=1 (perf)

@bors
Copy link
Contributor

bors commented Sep 24, 2017

⌛ Testing commit c10b23e with merge 93d48986d0b8c12cd2f29431485fee2ebccd0fca...

@bors
Copy link
Contributor

bors commented Sep 24, 2017

💔 Test failed - status-travis

@arielb1
Copy link
Contributor Author

arielb1 commented Sep 25, 2017

@bors r=eddyb

@bors
Copy link
Contributor

bors commented Sep 25, 2017

📌 Commit 7bb0923 has been approved by eddyb

@bors
Copy link
Contributor

bors commented Sep 25, 2017

⌛ Testing commit 7bb0923 with merge 91dbf52...

bors added a commit that referenced this pull request Sep 25, 2017
encode region::Scope using fewer bytes

Now that region::Scope is no longer interned, its size is more important. This PR encodes region::Scope in 8 bytes instead of 12, which should speed up region inference somewhat (perf testing needed) and should improve the margins on #36799 by 64MB (that's not a lot, I did this PR mostly to speed up region inference).

This is a perf-sensitive PR. Please don't roll me up.

r? @eddyb

This is based on  #44743 so I could get more accurate measurements on #36799.
@carols10cents carols10cents added the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label Sep 25, 2017
@bors
Copy link
Contributor

bors commented Sep 25, 2017

☀️ Test successful - status-appveyor, status-travis
Approved by: eddyb
Pushing 91dbf52 to master...

@bors bors merged commit 7bb0923 into rust-lang:master Sep 25, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants