rustc: Implement the #[global_allocator] attribute #42727

alexcrichton · 2017-06-18T01:53:19Z

This PR is an implementation of RFC 1974 which specifies a new method of
defining a global allocator for a program. This obsoletes the old
#![allocator] attribute and also removes support for it.

The new #[global_allocator] attribute solves many issues encountered with the
#![allocator] attribute such as composition and restrictions on the crate
graph itself. The compiler now has much more control over the ABI of the
allocator and how it's implemented, allowing much more freedom in terms of how
this feature is implemented.

cc #27389

rust-highfive · 2017-06-18T01:53:30Z

r? @brson

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

alexcrichton · 2017-06-18T01:54:12Z

Note that this is temporarily rebased on #42313 but will be rebased once that lands. Only the last commit here should be relevant.

r? @sfackler

sfackler · 2017-06-18T02:12:26Z

src/liballoc/heap.rs

-                self.dealloc(ptr, layout);
-            }
-            result
+            mem::forget(err);


Seems like we need a panic guard for the err, or can just not worry about it if it doesn't have a destructor.

An interesting point yeah. I believe, though, that it's actually memory unsafe for a global allocator to panic right now. This is asserted by the compiler and it would also be downright bad if we panicked-when panicking due to an assertion failure.

It may be best to just insert guards here to abort on panic, a global allocator should have no business panicking.

I've opted to use ManuallyDrop and added some words to the docs of #![global_allocator]

Mark-Simulacrum · 2017-06-18T03:40:02Z

I'm somewhat concerned with the approach taken here as I understand it of just removing the allocator attribute in the same PR as we add global_allocator. Is the transition extremely simple and painless? If so, then perhaps this isn't too big a problem but I personally would prefer that we kept allocator around for at least a little bit (even a half-cycle), though a full cycle would be better, I think.

alexcrichton · 2017-06-18T19:08:32Z

@Mark-Simulacrum I opted to just remove #[allocator] because keeping both features implemented would be significantly more work than what's implemented here. I suspect there are relatively few allocator crate (beyond the two in-tree) and what is external should be easy to migrate. While nightly is relatively stable I don't think it should be a hard constraint when we would otherwise bend over backwards.

Mark-Simulacrum · 2017-06-18T19:25:52Z

Ah, okay. Perhaps we could prepare some instructions on the migration? I assume it's relatively straightforward...

alexcrichton · 2017-06-18T20:36:53Z

Perhaps we could prepare some instructions on the migration?

Are you thinking of something more than https://github.com/alexcrichton/rust/blob/1389e1c4b713dfc9436c760eb6ffbde806f31c0f/src/doc/unstable-book/src/language-features/global-allocator.md?

Mark-Simulacrum · 2017-06-19T00:15:33Z

Yeah, that's good -- I was thinking about having a specific guide for moving to the new trait but no reason to in light of that, I guess. It seems better than what we had before, certainly -- I wasn't able to find documentation for the old attribute quickly.

bjorn3 · 2017-06-20T09:27:45Z

.gitmodules

@@ -1,6 +1,6 @@
 [submodule "src/llvm"]
 	path = src/llvm
-	url = https://github.com/rust-lang/llvm.git
+	url = https://github.com/alexcrichton/llvm.git


Should this be changed back?

Yes, I will do that after r+. This is currently necessary to pull in alexcrichton/llvm@fd2b33b

alexcrichton · 2017-06-20T14:15:43Z

Rebased now that #42313 has landed.

bors · 2017-06-21T08:47:44Z

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

The Alloc trait for allocators was added in Rust nightly 2017/06/21 (from rust-lang/rust#42313). With the addition of allowing a global allocator to be specified (pending in rust-lang/rust#42727) this will obviate the need for the alloc_uefi crate.

bors · 2017-06-22T00:37:48Z

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

alexcrichton · 2017-06-22T00:45:12Z

Rebased, ping r? @sfackler

rustc: Implement the #[global_allocator] attribute This PR is an implementation of [RFC 1974] which specifies a new method of defining a global allocator for a program. This obsoletes the old `#![allocator]` attribute and also removes support for it. [RFC 1974]: rust-lang/rfcs#1974 The new `#[global_allocator]` attribute solves many issues encountered with the `#![allocator]` attribute such as composition and restrictions on the crate graph itself. The compiler now has much more control over the ABI of the allocator and how it's implemented, allowing much more freedom in terms of how this feature is implemented. cc #27389

bors · 2017-07-06T02:34:19Z

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

pnkfelix · 2017-07-06T13:08:20Z

@alexcrichton wrote:

sure but the problem is that there's basically no cross-platform way to actually do that.

That's a good argument against leaving this test in place as it currently stands.

But can't we just make it conditional, so that it only runs on certain platforms (namely, the typical platforms used by compiler hackers)? That would probably satisfy the compiler hackers who once-a-year might otherwise need to create such a program, while keeping the maintenance burden in check?

pnkfelix · 2017-07-06T13:13:01Z

I wrote:

But can't we just make it conditional, so that it only runs on certain platforms

in fact, it seems that's exactly what we already do.

So I'd say that test has in fact served its purpose: Documenting the minimal code one needs to get a #[no_core] hello world up and running, a process which the diff here makes clear has certainly changed with this PR...

(But at the same time, I can appreciate that it was a pain for @alexcrichton to actually figure out the necesssary incantation here...)

alexcrichton · 2017-07-06T15:59:08Z

@pnkfelix ah yeah I'd be fine just adding a bunch of // ignore-* directives to only run it on Linux/OSX x86 where it shoudl be manageable

The Alloc trait for allocators was added in Rust nightly 2017/06/21 (from rust-lang/rust#42313). With the addition of allowing a global allocator to be specified (pending in rust-lang/rust#42727) this will obviate the need for the alloc_uefi crate.

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.

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.

rust-highfive assigned brson Jun 18, 2017

rust-highfive assigned sfackler and unassigned brson Jun 18, 2017

sfackler reviewed Jun 18, 2017

View reviewed changes

alexcrichton force-pushed the allocators-new branch from 1c74c13 to 1389e1c Compare June 18, 2017 20:35

carols10cents added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jun 19, 2017

bjorn3 requested changes Jun 20, 2017

View reviewed changes

alexcrichton force-pushed the allocators-new branch from 1389e1c to a1f7527 Compare June 20, 2017 14:15

This was referenced Jun 20, 2017

Tracking issue for changing the global, default allocator (RFC 1974) #27389

Closed

Allocator traits and std::heap #32838

Open

alexcrichton force-pushed the allocators-new branch from a1f7527 to 31e3af1 Compare June 20, 2017 18:17

alexcrichton force-pushed the allocators-new branch from 31e3af1 to b470620 Compare June 21, 2017 14:02

alexcrichton force-pushed the allocators-new branch from b470620 to 8ab7ca9 Compare June 22, 2017 00:44

alexcrichton closed this Jun 22, 2017

alexcrichton reopened this Jun 22, 2017

bors merged commit 695dee0 into rust-lang:master Jul 6, 2017

alexcrichton deleted the allocators-new branch July 6, 2017 02:38

bors mentioned this pull request Jul 6, 2017

Add ‘#[serial]’ tests (v2) #42684

Closed

phil-opp mentioned this pull request Jul 7, 2017

Use new allocator API rust-osdev/linked-list-allocator#4

Merged

jemtucker mentioned this pull request Jul 10, 2017

Replace custom allocator with 'global_allocator' jemtucker/opsys#19

Closed

alexcrichton mentioned this pull request Jul 17, 2017

jemalloc still pulled in with extern crate alloc_system on nightlies >= 2017-07-07 #43277

Closed

f-bro mentioned this pull request Jul 19, 2017

Update alloc API to latest nightly rust-embedded/embedded-alloc#6

Closed

f-bro mentioned this pull request Aug 3, 2017

Update alloc API to latest nightly rust-embedded/embedded-alloc#7

Merged

alexcrichton added a commit to alexcrichton/rust that referenced this pull request Aug 11, 2017

std: Tag OOM functions as #[cold]

fae60b3

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.

alexcrichton mentioned this pull request Aug 11, 2017

Optimize allocation paths in RawVec #43815

Merged

TimNN mentioned this pull request Sep 8, 2017

[Regression] Rust v1.20.0 leads to segfault with lazy_static Mutex inside a dylib #44283

Closed

hanna-kruppe mentioned this pull request Sep 13, 2017

Reloading a dylib stops working when moving from 1.19.0 to 1.20.0 #44365

Closed

juleskers mentioned this pull request Jan 3, 2018

No warning for obsolete allocator syntax #47143

Closed

arielb1 mentioned this pull request Jan 23, 2018

Performance regressions of compiled code over the last year #47561

Open

This was referenced May 29, 2018

OOM handling changes #50880

Merged

Tracking issue for the OOM hook (alloc_error_hook) #51245

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

rustc: Implement the #[global_allocator] attribute #42727

rustc: Implement the #[global_allocator] attribute #42727

alexcrichton commented Jun 18, 2017 •

edited by arielb1

Loading

rust-highfive commented Jun 18, 2017

alexcrichton commented Jun 18, 2017

sfackler Jun 18, 2017

alexcrichton Jun 18, 2017

alexcrichton Jun 18, 2017

Mark-Simulacrum commented Jun 18, 2017

alexcrichton commented Jun 18, 2017

Mark-Simulacrum commented Jun 18, 2017

alexcrichton commented Jun 18, 2017

Mark-Simulacrum commented Jun 19, 2017

bjorn3 Jun 20, 2017

alexcrichton Jun 20, 2017

alexcrichton commented Jun 20, 2017

bors commented Jun 21, 2017

bors commented Jun 22, 2017

alexcrichton commented Jun 22, 2017

bors commented Jul 6, 2017

pnkfelix commented Jul 6, 2017

pnkfelix commented Jul 6, 2017

alexcrichton commented Jul 6, 2017

rustc: Implement the #[global_allocator] attribute #42727

rustc: Implement the #[global_allocator] attribute #42727

Conversation

alexcrichton commented Jun 18, 2017 • edited by arielb1 Loading

rust-highfive commented Jun 18, 2017

alexcrichton commented Jun 18, 2017

sfackler Jun 18, 2017

Choose a reason for hiding this comment

alexcrichton Jun 18, 2017

Choose a reason for hiding this comment

alexcrichton Jun 18, 2017

Choose a reason for hiding this comment

Mark-Simulacrum commented Jun 18, 2017

alexcrichton commented Jun 18, 2017

Mark-Simulacrum commented Jun 18, 2017

alexcrichton commented Jun 18, 2017

Mark-Simulacrum commented Jun 19, 2017

bjorn3 Jun 20, 2017

Choose a reason for hiding this comment

alexcrichton Jun 20, 2017

Choose a reason for hiding this comment

alexcrichton commented Jun 20, 2017

bors commented Jun 21, 2017

bors commented Jun 22, 2017

alexcrichton commented Jun 22, 2017

bors commented Jul 6, 2017

pnkfelix commented Jul 6, 2017

pnkfelix commented Jul 6, 2017

alexcrichton commented Jul 6, 2017

alexcrichton commented Jun 18, 2017 •

edited by arielb1

Loading