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

Optimize allocation paths in RawVec #43815

Merged
merged 3 commits into from
Aug 13, 2017
Merged

Conversation

alexcrichton
Copy link
Member

Since the Alloc trait was introduced (#42313) and it was integrated everywhere (#42727) there's been some slowdowns and regressions that have slipped through. The intention of this PR is to try to tackle at least some of them, but they've been very difficult to quantify up to this point so it probably doesn't solve everything.

This PR primarily targets the RawVec type, specifically the double function. The codegen for this function is now much closer to what it was before #42313 landed as many runtime checks have been elided.

None of these require a significant amount of code and using `#[inline]` will
allow constructors to get inlined, improving codegen at allocation callsites.
This was forgotten from rust-lang#42727 by accident, but these functions are rarely
called and codegen can be improved in LLVM with the `#[cold]` tag.
@rust-highfive
Copy link
Collaborator

r? @sfackler

(rust_highfive has picked a reviewer for you, use r? to override)

@alexcrichton
Copy link
Member Author

I'd like to nominate this for beta eventually, but I'd prefer to wait and see the impact on perf.rust-lang.org before deciding to do so.

@kennytm
Copy link
Member

kennytm commented Aug 12, 2017

"fatal runtime error: unsupported allocator request"

[00:55:55]      Running build/x86_64-unknown-linux-gnu/stage1-std/x86_64-unknown-linux-gnu/release/deps/collectionstests-9d97829c4a885d0c
[00:55:55] 
[00:55:55] running 461 tests
[00:55:55] thread '<unnamed>' panicked at 'explicit panic', /checkout/src/liballoc/../liballoc/tests/slice.rs:1194:16
[00:55:55] fatal runtime error: unsupported allocator request
[00:55:55] error: An unknown error occurred

/src/liballoc/tests/slice.rs:1194

use core::slice;
use heap::Heap;
use heap::{Alloc, Layout, Heap, AllocErr};
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cannot build tidy due to unused import.

[00:02:33]    Compiling alloc v0.0.0 (file:///checkout/src/liballoc)
[00:02:36] error: unused import: `AllocErr`
[00:02:36]   --> /checkout/src/liballoc/raw_vec.rs:16:33
[00:02:36]    |
[00:02:36] 16 | use heap::{Alloc, Layout, Heap, AllocErr};
[00:02:36]    |                                 ^^^^^^^^
[00:02:36]    |
[00:02:36] note: lint level defined here
[00:02:36]   --> /checkout/src/liballoc/lib.rs:77:9
[00:02:36]    |
[00:02:36] 77 | #![deny(warnings)]
[00:02:36]    |         ^^^^^^^^
[00:02:36]    = note: #[deny(unused_imports)] implied by #[deny(warnings)]
[00:02:36] 
[00:02:37] error: aborting due to previous error
[00:02:37] 
[00:02:37] error: Could not compile `alloc`.

The `RawVec` type has a number of invariants that it upholds throughout its
execution, and as a result many of the runtime checks imposed by using `Layout`
in a "raw" fashion aren't actually necessary. For example a `RawVec`'s capacity
is intended to always match the layout which "fits" the allocation, so we don't
need any runtime checks when retrieving the current `Layout` for a vector.
Consequently, this adds a safe `current_layout` function which internally uses
the `from_size_align_unchecked` function.

Along the same lines we know that most construction of new layouts will not
overflow. All allocations in `RawVec` are kept below `isize::MAX` and valid
alignments are also kept low enough that we're guaranteed that `Layout` for a
doubled vector will never overflow and will always succeed construction.
Consequently a few locations can use `from_size_align_unchecked` in addition
when constructing the *new* layout to allocate (or reallocate), which allows for
eliding some more runtime checks.

Overall this should significant improve performance for an important function,
`RawVec::double`. This commit removes four runtime jumps before `__rust_realloc`
is called, as well as one after it's called.
@alexcrichton
Copy link
Member Author

Green!

@alexcrichton alexcrichton added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Aug 12, 2017
@sfackler
Copy link
Member

@bors r+

@bors
Copy link
Contributor

bors commented Aug 12, 2017

📌 Commit 3a83165 has been approved by sfackler

@bors
Copy link
Contributor

bors commented Aug 13, 2017

⌛ Testing commit 3a83165 with merge ab40a7c...

bors added a commit that referenced this pull request Aug 13, 2017
Optimize allocation paths in RawVec

Since the `Alloc` trait was introduced (#42313) and it was integrated everywhere (#42727) there's been some slowdowns and regressions that have slipped through. The intention of this PR is to try to tackle at least some of them, but they've been very difficult to quantify up to this point so it probably doesn't solve everything.

This PR primarily targets the `RawVec` type, specifically the `double` function. The codegen for this function is now much closer to what it was before #42313 landed as many runtime checks have been elided.
@bors
Copy link
Contributor

bors commented Aug 13, 2017

☀️ Test successful - status-appveyor, status-travis
Approved by: sfackler
Pushing ab40a7c to master...

@bors bors merged commit 3a83165 into rust-lang:master Aug 13, 2017
@alexcrichton alexcrichton deleted the optimize-alloc branch August 22, 2017 19:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants