Skip to content

Conversation

brson
Copy link
Contributor

@brson brson commented Jul 1, 2018

On nightly/beta the global allocator API is enabled, but jemalloc is still the default. This does not seem to be indicated in the documentation though. This change makes it clear that jemalloc is the default allocator for executables on Unixes and shows how to instead enable the system allocator.

If this pull is accepted it should be backported to beta. Somebody please tag it as appropriate.

r? @steveklabnik

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jul 1, 2018
@Mark-Simulacrum Mark-Simulacrum added the beta-nominated Nominated for backporting to the compiler in the beta channel. label Jul 1, 2018
@brson
Copy link
Contributor Author

brson commented Jul 1, 2018

Here's a PR against the reference: rust-lang/reference#367

@rust-highfive
Copy link
Contributor

The job x86_64-gnu-llvm-3.9 of your PR failed on Travis (raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log.
[01:16:23] 
[01:16:23]    Doc-tests std
[01:16:25] 
[01:16:25] running 931 tests
[01:16:58] iiFi................................................................................................
[01:17:29] ......i......i...i......i...........................................................................
[01:17:38] ....................................................................................................
[01:17:54] .....................iiii........ii.................................................................
[01:18:03] ....................................................................................................
---
[01:18:59] ---- alloc.rs - alloc (line 28) stdout ----
[01:18:59] error[E0432]: unresolved import `super::GLOBAL`
[01:18:59]  --> alloc.rs:30:1
[01:18:59]   |
[01:18:59] 5 | static GLOBAL: std::alloc::System = std::alloc::System;
[01:18:59]   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ no `GLOBAL` in the root
[01:18:59] 
[01:18:59] thread 'alloc.rs - alloc (line 28)' panicked at 'assertion failed: prev.is_none()', librustc/middle/region.rs:509:13
[01:18:59] thread 'alloc.rs - alloc (line 28)' panicked at 'couldn't compile the test', librustdoc/test.rs:327:17
[01:18:59] 
[01:18:59] 
[01:18:59] failures:
[01:18:59] failures:
[01:18:59]     alloc.rs - alloc (line 28)
[01:18:59] 
[01:18:59] test result: FAILED. 906 passed; 1 failed; 24 ignored; 0 measured; 0 filtered out
[01:18:59] 
[01:18:59] error: test failed, to rerun pass '--doc'
[01:18:59] 
[01:18:59] 
[01:18:59] command did not execute successfully: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0/bin/cargo" "test" "--target" "x86_64-unknown-linux-gnu" "-j" "4" "--release" "--locked" "--color" "always" "--features" "panic-unwind jemalloc backtrace" "--manifest-path" "/checkout/src/libstd/Cargo.toml" "-p" "std" "--" "--quiet"
[01:18:59] 
[01:18:59] 
[01:18:59] failed to run: /checkout/obj/build/bootstrap/debug/bootstrap test
[01:18:59] Build completed unsuccessfully in 0:36:34
[01:18:59] Build completed unsuccessfully in 0:36:34
[01:18:59] make: *** [check] Error 1
[01:18:59] Makefile:58: recipe for target 'check' failed

The command "stamp sh -x -c "$RUN_SCRIPT"" exited with 2.
travis_time:start:182bbb78
$ date && (curl -fs --head https://google.com | grep ^Date: | sed 's/Date: //g' || true)

I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact @TimNN. (Feature Requests)

@steveklabnik
Copy link
Contributor

@rust-lang/libs, is this something we want to do? my understanding is that

-//! Currently the default global allocator is unspecified.

because we want to switch to the system allocator.

@SimonSapin
Copy link
Contributor

I think it’s fine to document the current reality as long as docs are clear that it is unspecified and expected to change in future versions. So rather than a note at the bottom I’d prefer leading with that, and then explaining what is current.

@alexcrichton
Copy link
Member

I'd agree with @SimonSapin in that it should be fine to document reality so long as there's a lot of notes and caveats saying so and that it's subject to change (perhaps with links to tracking issues)

@stokhos stokhos added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 6, 2018
@stokhos
Copy link

stokhos commented Jul 6, 2018

Ping from triage @brson, any progress on this PR?

@pietroalbini pietroalbini added the T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. label Jul 12, 2018
@TimNN
Copy link
Contributor

TimNN commented Jul 17, 2018

Ping from triage, @brson: What is the status of this PR?

@pietroalbini
Copy link
Member

Ping @brson, 1.28 will be released in a week, can you make the requested changes soon?

@pietroalbini pietroalbini removed the beta-nominated Nominated for backporting to the compiler in the beta channel. label Jul 30, 2018
@TimNN
Copy link
Contributor

TimNN commented Aug 7, 2018

Ping from triage @brson! We haven't heard from you in a while so I'm closing this PR for now. Please re-open it when you have time to work on it again.

@TimNN TimNN closed this Aug 7, 2018
@TimNN TimNN added S-inactive Status: Inactive and waiting on the author. This is often applied to closed PRs. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Aug 7, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-inactive Status: Inactive and waiting on the author. This is often applied to closed PRs. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants