From bb23bfc2cde4b56372ac7324d0bdec2232a162e5 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Thu, 23 Dec 2021 15:47:32 +1100 Subject: [PATCH] Remove useless `#[global_allocator]` from rustc and rustdoc. This was added in #83152, which has several errors in its comments. This commit also fix up the comments, which are quite wrong and misleading. --- compiler/rustc/src/main.rs | 41 ++++++++++++++++++++++---------------- src/librustdoc/lib.rs | 18 +++-------------- 2 files changed, 27 insertions(+), 32 deletions(-) diff --git a/compiler/rustc/src/main.rs b/compiler/rustc/src/main.rs index c80fab99496bc..4edd095af10b5 100644 --- a/compiler/rustc/src/main.rs +++ b/compiler/rustc/src/main.rs @@ -1,25 +1,32 @@ -// Configure jemalloc as the `global_allocator` when configured. This is -// so that we use the sized deallocation apis jemalloc provides -// (namely `sdallocx`). +// A note about jemalloc: rustc uses jemalloc when built for CI and +// distribution. The obvious way to do this is with the `#[global_allocator]` +// mechanism. However, for complicated reasons (see +// https://github.com/rust-lang/rust/pull/81782#issuecomment-784438001 for some +// details) that mechanism doesn't work here. Also, we must use a consistent +// allocator across the rustc <-> llvm boundary, and `#[global_allocator]` +// wouldn't provide that. // -// The symbol overrides documented below are also performed so that we can -// ensure that we use a consistent allocator across the rustc <-> llvm boundary -#[cfg(feature = "jemalloc")] -#[global_allocator] -static ALLOC: tikv_jemallocator::Jemalloc = tikv_jemallocator::Jemalloc; - +// Instead, we use a lower-level mechanism. rustc is linked with jemalloc in a +// way such that jemalloc's implementation of `malloc`, `free`, etc., override +// the libc allocator's implementation. This means that Rust's `System` +// allocator, which calls `libc::malloc()` et al., is actually calling into +// jemalloc. +// +// A consequence of not using `GlobalAlloc` (and the `tikv-jemallocator` crate +// provides an impl of that trait, which is called `Jemalloc`) is that we +// cannot use the sized deallocation APIs (`sdallocx`) that jemalloc provides. +// It's unclear how much performance is lost because of this. +// +// As for the symbol overrides in `main` below: we're pulling in a static copy +// of jemalloc. We need to actually reference its symbols for it to get linked. +// The two crates we link to here, `std` and `rustc_driver`, are both dynamic +// libraries. So we must reference jemalloc symbols one way or another, because +// this file is the only object code in the rustc executable. #[cfg(feature = "tikv-jemalloc-sys")] use tikv_jemalloc_sys as jemalloc_sys; fn main() { - // Pull in jemalloc when enabled. - // - // Note that we're pulling in a static copy of jemalloc which means that to - // pull it in we need to actually reference its symbols for it to get - // linked. The two crates we link to here, std and rustc_driver, are both - // dynamic libraries. That means to pull in jemalloc we actually need to - // reference allocation symbols one way or another (as this file is the only - // object code in the rustc executable). + // See the comment at the top of this file for an explanation of this. #[cfg(feature = "tikv-jemalloc-sys")] { use std::os::raw::{c_int, c_void}; diff --git a/src/librustdoc/lib.rs b/src/librustdoc/lib.rs index 8699ab20b19d4..4197c7012f493 100644 --- a/src/librustdoc/lib.rs +++ b/src/librustdoc/lib.rs @@ -63,14 +63,12 @@ extern crate rustc_trait_selection; extern crate rustc_typeck; extern crate test; +// See docs in https://github.com/rust-lang/rust/blob/master/compiler/rustc/src/main.rs +// about jemalloc. #[cfg(feature = "jemalloc")] extern crate tikv_jemalloc_sys; #[cfg(feature = "jemalloc")] use tikv_jemalloc_sys as jemalloc_sys; -#[cfg(feature = "jemalloc")] -extern crate tikv_jemallocator; -#[cfg(feature = "jemalloc")] -use tikv_jemallocator as jemallocator; use std::default::Default; use std::env; @@ -125,15 +123,9 @@ mod visit; mod visit_ast; mod visit_lib; -// See docs in https://github.com/rust-lang/rust/blob/master/compiler/rustc/src/main.rs -// about jemallocator -#[cfg(feature = "jemalloc")] -#[global_allocator] -static ALLOC: jemallocator::Jemalloc = jemallocator::Jemalloc; - pub fn main() { // See docs in https://github.com/rust-lang/rust/blob/master/compiler/rustc/src/main.rs - // about jemalloc-sys + // about jemalloc. #[cfg(feature = "jemalloc")] { use std::os::raw::{c_int, c_void}; @@ -152,10 +144,6 @@ pub fn main() { #[used] static _F6: unsafe extern "C" fn(*mut c_void) = jemalloc_sys::free; - // On OSX, jemalloc doesn't directly override malloc/free, but instead - // registers itself with the allocator's zone APIs in a ctor. However, - // the linker doesn't seem to consider ctors as "used" when statically - // linking, so we need to explicitly depend on the function. #[cfg(target_os = "macos")] { extern "C" {