Skip to content

Sized fastpath #26023

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 2 commits into from
Jun 5, 2015
Merged

Sized fastpath #26023

merged 2 commits into from
Jun 5, 2015

Conversation

arielb1
Copy link
Contributor

@arielb1 arielb1 commented Jun 5, 2015

r? @eddyb

The change to trans::common::type_is_sized is because we currently abort, rather than return random results, on overflow.

This seems to improve performance by the same 2-3% of my selection
fast-path.
@eddyb
Copy link
Member

eddyb commented Jun 5, 2015

AFAICT there is no chance of leakage of cached results that depend on the parameter environment, though there might be other gotchas I'm not aware of.
LGTM but I'd really have r? @nikomatsakis on this.

@rust-highfive rust-highfive assigned nikomatsakis and unassigned eddyb Jun 5, 2015
@arielb1
Copy link
Contributor Author

arielb1 commented Jun 5, 2015

I am using the same caching logic as before.

@eddyb
Copy link
Member

eddyb commented Jun 5, 2015

You're totally right, my bad. @bors r+

@bors
Copy link
Collaborator

bors commented Jun 5, 2015

📌 Commit 595409d has been approved by eddyb

@arielb1
Copy link
Contributor Author

arielb1 commented Jun 5, 2015

Performance numbers:

Before:
581.72user 4.75system 7:42.74elapsed 126%CPU (0avgtext+0avgdata 1176224maxresident)k
llvm took 359.183

After:
573.72user 4.49system 7:34.47elapsed 127%CPU (0avgtext+0avgdata 1166328maxresident)k
llvm took 356.015

@eddyb
Copy link
Member

eddyb commented Jun 5, 2015

@arielb1 is that saying it took... longer? @bors r-
EDIT: I've reverted the r-, but do note what @rust-highfive did: it matched "longer?" as "r?".
cc @nrc

@rust-highfive rust-highfive assigned bors and unassigned nikomatsakis Jun 5, 2015
@arielb1
Copy link
Contributor Author

arielb1 commented Jun 5, 2015

I mixed them up

@eddyb
Copy link
Member

eddyb commented Jun 5, 2015

@bors r+

@bors
Copy link
Collaborator

bors commented Jun 5, 2015

📌 Commit 39e6855 has been approved by eddyb

@bors
Copy link
Collaborator

bors commented Jun 5, 2015

⌛ Testing commit 39e6855 with merge 75fb009...

bors added a commit that referenced this pull request Jun 5, 2015
r? @eddyb 

The change to `trans::common::type_is_sized` is because we currently abort, rather than return random results, on overflow.
@bors bors merged commit 39e6855 into rust-lang:master Jun 5, 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.

4 participants