Skip to content

Simplify jemalloc setup (without perf regression) #148925

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.

Sign up for GitHub

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
+37 −161
Open

Simplify jemalloc setup (without perf regression) #148925

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions compiler/rustc/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,9 @@ wasi = "=0.14.2"


[dependencies.tikv-jemalloc-sys]
version = "0.6.0"
version = "0.6.1"
optional = true
features = ['unprefixed_malloc_on_supported_platforms']
features = ['override_allocator_on_supported_platforms']

[features]
# tidy-alphabetical-start
Expand Down
63 changes: 14 additions & 49 deletions compiler/rustc/src/main.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,26 +7,25 @@
// 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.
// details) that mechanism doesn't work here. Also, we'd like to use a
// consistent allocator across the rustc <-> llvm boundary, and
// `#[global_allocator]` wouldn't provide that.
//
// 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.
// Instead, we use a lower-level mechanism, namely the
// `"override_allocator_on_supported_platforms"` Cargo feature of jemalloc-sys.
//
// This makes jemalloc-sys override the libc/system allocator's implementation
// of `malloc`, `free`, etc.. 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.
// NOTE: Even though Cargo passes `--extern` with `tikv_jemalloc_sys`, we still need to `use` the
// crate for the compiler to see the `#[used]`, see https://github.com/rust-lang/rust/issues/64402.
// This is similarly required if we used a crate with `#[global_allocator]`.
//
// NOTE: if you are reading this comment because you want to set a custom `global_allocator` for
// benchmarking, consider using the benchmarks in the `rustc-perf` collector suite instead:
Expand All @@ -36,43 +35,9 @@
// to compare their performance, see
// https://github.com/rust-lang/rust/commit/b90cfc887c31c3e7a9e6d462e2464db1fe506175#diff-43914724af6e464c1da2171e4a9b6c7e607d5bc1203fa95c0ab85be4122605ef
// for an example of how to do so.
#[cfg(feature = "jemalloc")]
use tikv_jemalloc_sys as _;

fn main() {
// See the comment at the top of this file for an explanation of this.
#[cfg(feature = "jemalloc")]
{
use std::os::raw::{c_int, c_void};

use tikv_jemalloc_sys as jemalloc_sys;

#[used]
static _F1: unsafe extern "C" fn(usize, usize) -> *mut c_void = jemalloc_sys::calloc;
#[used]
static _F2: unsafe extern "C" fn(*mut *mut c_void, usize, usize) -> c_int =
jemalloc_sys::posix_memalign;
#[used]
static _F3: unsafe extern "C" fn(usize, usize) -> *mut c_void = jemalloc_sys::aligned_alloc;
#[used]
static _F4: unsafe extern "C" fn(usize) -> *mut c_void = jemalloc_sys::malloc;
#[used]
static _F5: unsafe extern "C" fn(*mut c_void, usize) -> *mut c_void = jemalloc_sys::realloc;
#[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")]
{
unsafe extern "C" {
fn _rjem_je_zone_register();
}

#[used]
static _F7: unsafe extern "C" fn() = _rjem_je_zone_register;
}
}

rustc_driver::main()
}
41 changes: 7 additions & 34 deletions src/librustdoc/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,10 +61,14 @@ extern crate rustc_target;
extern crate rustc_trait_selection;
extern crate test;

// See docs in https://github.com/rust-lang/rust/blob/HEAD/compiler/rustc/src/main.rs
// about jemalloc.
/// See docs in https://github.com/rust-lang/rust/blob/HEAD/compiler/rustc/src/main.rs
/// and https://github.com/rust-lang/rust/pull/146627 for why we need this.
///
/// FIXME(madsmtm): This is loaded from the sysroot that was built with the other `rustc` crates
/// above, instead of via Cargo as you'd normally do. This is currently needed for LTO due to
/// https://github.com/rust-lang/cc-rs/issues/1613.
#[cfg(feature = "jemalloc")]
extern crate tikv_jemalloc_sys as jemalloc_sys;
extern crate tikv_jemalloc_sys as _;

use std::env::{self, VarError};
use std::io::{self, IsTerminal};
Expand Down Expand Up @@ -124,37 +128,6 @@ mod visit_ast;
mod visit_lib;

pub fn main() {
// See docs in https://github.com/rust-lang/rust/blob/HEAD/compiler/rustc/src/main.rs
// about jemalloc.
#[cfg(feature = "jemalloc")]
{
use std::os::raw::{c_int, c_void};

#[used]
static _F1: unsafe extern "C" fn(usize, usize) -> *mut c_void = jemalloc_sys::calloc;
#[used]
static _F2: unsafe extern "C" fn(*mut *mut c_void, usize, usize) -> c_int =
jemalloc_sys::posix_memalign;
#[used]
static _F3: unsafe extern "C" fn(usize, usize) -> *mut c_void = jemalloc_sys::aligned_alloc;
#[used]
static _F4: unsafe extern "C" fn(usize) -> *mut c_void = jemalloc_sys::malloc;
#[used]
static _F5: unsafe extern "C" fn(*mut c_void, usize) -> *mut c_void = jemalloc_sys::realloc;
#[used]
static _F6: unsafe extern "C" fn(*mut c_void) = jemalloc_sys::free;

#[cfg(target_os = "macos")]
{
unsafe extern "C" {
fn _rjem_je_zone_register();
}

#[used]
static _F7: unsafe extern "C" fn() = _rjem_je_zone_register;
}
}

let mut early_dcx = EarlyDiagCtxt::new(ErrorOutputType::default());

rustc_driver::install_ice_hook(
Expand Down
40 changes: 7 additions & 33 deletions src/tools/clippy/src/driver.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,14 @@ extern crate rustc_interface;
extern crate rustc_session;
extern crate rustc_span;

// See docs in https://github.com/rust-lang/rust/blob/HEAD/compiler/rustc/src/main.rs
// about jemalloc.
/// See docs in https://github.com/rust-lang/rust/blob/HEAD/compiler/rustc/src/main.rs
/// and https://github.com/rust-lang/rust/pull/146627 for why we need this.
///
/// FIXME(madsmtm): This is loaded from the sysroot that was built with the other `rustc` crates
/// above, instead of via Cargo as you'd normally do. This is currently needed for LTO due to
/// https://github.com/rust-lang/cc-rs/issues/1613.
#[cfg(feature = "jemalloc")]
extern crate tikv_jemalloc_sys as jemalloc_sys;
extern crate tikv_jemalloc_sys as _;

use clippy_utils::sym;
use declare_clippy_lint::LintListBuilder;
Expand Down Expand Up @@ -189,36 +193,6 @@ const BUG_REPORT_URL: &str = "https://github.com/rust-lang/rust-clippy/issues/ne

#[expect(clippy::too_many_lines)]
pub fn main() {
// See docs in https://github.com/rust-lang/rust/blob/HEAD/compiler/rustc/src/main.rs
// about jemalloc.
#[cfg(feature = "jemalloc")]
{
use std::os::raw::{c_int, c_void};

#[used]
static _F1: unsafe extern "C" fn(usize, usize) -> *mut c_void = jemalloc_sys::calloc;
#[used]
static _F2: unsafe extern "C" fn(*mut *mut c_void, usize, usize) -> c_int = jemalloc_sys::posix_memalign;
#[used]
static _F3: unsafe extern "C" fn(usize, usize) -> *mut c_void = jemalloc_sys::aligned_alloc;
#[used]
static _F4: unsafe extern "C" fn(usize) -> *mut c_void = jemalloc_sys::malloc;
#[used]
static _F5: unsafe extern "C" fn(*mut c_void, usize) -> *mut c_void = jemalloc_sys::realloc;
#[used]
static _F6: unsafe extern "C" fn(*mut c_void) = jemalloc_sys::free;

#[cfg(target_os = "macos")]
{
unsafe extern "C" {
fn _rjem_je_zone_register();
}

#[used]
static _F7: unsafe extern "C" fn() = _rjem_je_zone_register;
}
}

let early_dcx = EarlyDiagCtxt::new(ErrorOutputType::default());

rustc_driver::init_rustc_env_logger(&early_dcx);
Expand Down
4 changes: 2 additions & 2 deletions src/tools/miri/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,8 @@ serde_json = { version = "1.0", optional = true }
# But only for some targets, it fails for others. Rustc configures this in its CI, but we can't
# easily use that since we support of-tree builds.
[target.'cfg(any(target_os = "linux", target_os = "macos"))'.dependencies.tikv-jemalloc-sys]
version = "0.6.0"
features = ['unprefixed_malloc_on_supported_platforms']
version = "0.6.1"
features = ['override_allocator_on_supported_platforms']

[target.'cfg(unix)'.dependencies]
libc = "0.2"
Expand Down
46 changes: 5 additions & 41 deletions src/tools/miri/src/bin/miri.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,11 @@ extern crate rustc_middle;
extern crate rustc_session;
extern crate rustc_span;

/// See docs in https://github.com/rust-lang/rust/blob/HEAD/compiler/rustc/src/main.rs
/// and https://github.com/rust-lang/rust/pull/146627 for why we need this `use` statement.
#[cfg(any(target_os = "linux", target_os = "macos"))]
use tikv_jemalloc_sys as _;
Copy link
Member

@lqd lqd Nov 14, 2025

Choose a reason for hiding this comment

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

would extern crate be needed here too instead of the use?

Copy link
Contributor Author

@madsmtm madsmtm Nov 15, 2025

Choose a reason for hiding this comment

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

Nope, miri already uses tikv_jemalloc_sys from Cargo, so extern crate and use work the same here.

Copy link
Member

@RalfJung RalfJung Nov 15, 2025

Choose a reason for hiding this comment

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

Does that mean Miri suffers from the cc-rs issue? The comment in clippy seems to say that the extern crate thing is needed to avoid the cc-rs issue.

Copy link
Contributor Author

@madsmtm madsmtm Nov 23, 2025

Choose a reason for hiding this comment

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

Does that mean Miri suffers from the cc-rs issue? The comment in clippy seems to say that the extern crate thing is needed to avoid the cc-rs issue.

I haven't tested it, but on reflection I'm fairly sure that it does, yeah.

Copy link
Member

@RalfJung RalfJung Nov 23, 2025

Choose a reason for hiding this comment

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

Uh then maybe let's not land this as-is? Please don't break Miri.^^

Copy link
Contributor Author

@madsmtm madsmtm Nov 23, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

As in, jemalloc isn't LTO optimized for the miri Rustup component, neither before nor after this PR.

But jemalloc is used both before and after.

Copy link
Member

@Kobzol Kobzol Nov 23, 2025

Choose a reason for hiding this comment

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

FWIW, LTO for jemalloc resulted maybe in a ~1% total instruction count win for rustc, IIRC, it wasn't a huge deal.

Copy link
Member

@RalfJung RalfJung Nov 23, 2025

Choose a reason for hiding this comment

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

Ah okay. I'd appreciate your help with fixing that (if only just to have things consistent across tools), but it doesn't have to be in this PR then.

Copy link
Contributor Author

@madsmtm madsmtm Nov 23, 2025

Choose a reason for hiding this comment

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

May be a bit troublesome, because Miri supports being compiled outside the rust-lang/rust workspace (?)

If that's not an issue, then it'd be enough to just do:

diff --git a/src/tools/miri/Cargo.toml b/src/tools/miri/Cargo.toml
index 611e549930a..2235203e2d7 100644
--- a/src/tools/miri/Cargo.toml
+++ b/src/tools/miri/Cargo.toml
@@ -29,13 +29,6 @@ directories = "6"
 bitflags = "2.6"
 serde_json = { version = "1.0", optional = true }
 
-# Copied from `compiler/rustc/Cargo.toml`.
-# But only for some targets, it fails for others. Rustc configures this in its CI, but we can't
-# easily use that since we support of-tree builds.
-[target.'cfg(any(target_os = "linux", target_os = "macos"))'.dependencies.tikv-jemalloc-sys]
-version = "0.6.1"
-features = ['override_allocator_on_supported_platforms']
-
 [target.'cfg(unix)'.dependencies]
 libc = "0.2"
 # native-lib dependencies
@@ -75,6 +68,7 @@ stack-cache = []
 expensive-consistency-checks = ["stack-cache"]
 tracing = ["serde_json"]
 native-lib = ["dep:libffi", "dep:libloading", "dep:capstone", "dep:ipc-channel", "dep:nix", "dep:serde"]
+jemalloc = []
 
 [lints.rust.unexpected_cfgs]
 level = "warn"
diff --git a/src/tools/miri/src/bin/miri.rs b/src/tools/miri/src/bin/miri.rs
index d7c5cb68e4f..cab31d159d3 100644
--- a/src/tools/miri/src/bin/miri.rs
+++ b/src/tools/miri/src/bin/miri.rs
@@ -22,8 +22,12 @@
 
 /// See docs in https://github.com/rust-lang/rust/blob/HEAD/compiler/rustc/src/main.rs
 /// and https://github.com/rust-lang/rust/pull/146627 for why we need this `use` statement.
-#[cfg(any(target_os = "linux", target_os = "macos"))]
-use tikv_jemalloc_sys as _;
+///
+/// FIXME(madsmtm): This is loaded from the sysroot that was built with the other `rustc` crates
+/// above, instead of via Cargo as you'd normally do. This is currently needed for LTO due to
+/// https://github.com/rust-lang/cc-rs/issues/1613.
+#[cfg(feature = "jemalloc")]
+extern crate tikv_jemalloc_sys as _;
 
 mod log;

I can put up a PR, but I'm unfamiliar with how CI and perf runs in r-l/rust vs. r-l/miri, so I wouldn't know which repo to target? And I'd fear breaking jemalloc for a use-case that I don't know about. And besides, I'm kinda hoping to resolve the cc-rs issue, in which case we wouldn't have to do anything in Miri.

Copy link
Member

@RalfJung RalfJung Nov 23, 2025

Choose a reason for hiding this comment

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

May be a bit troublesome, because Miri supports being compiled outside the rust-lang/rust workspace (?)

That's also the case for clippy though.

It seems the main difference is using tikv-jemalloc-sys from the sysroot vs having it as a dependency directly in the crate. And if it's already in the sysroot then it seems reasonable to use that, and avoid any risk of it being duplicated or so.


mod log;

use std::env;
Expand Down Expand Up @@ -395,48 +400,7 @@ fn parse_range(val: &str) -> Result<Range<u32>, &'static str> {
Ok(from..to)
}

#[cfg(any(target_os = "linux", target_os = "macos"))]
fn jemalloc_magic() {
// These magic runes are copied from
// <https://github.com/rust-lang/rust/blob/e89bd9428f621545c979c0ec686addc6563a394e/compiler/rustc/src/main.rs#L39>.
// See there for further comments.
use std::os::raw::{c_int, c_void};

use tikv_jemalloc_sys as jemalloc_sys;

#[used]
static _F1: unsafe extern "C" fn(usize, usize) -> *mut c_void = jemalloc_sys::calloc;
#[used]
static _F2: unsafe extern "C" fn(*mut *mut c_void, usize, usize) -> c_int =
jemalloc_sys::posix_memalign;
#[used]
static _F3: unsafe extern "C" fn(usize, usize) -> *mut c_void = jemalloc_sys::aligned_alloc;
#[used]
static _F4: unsafe extern "C" fn(usize) -> *mut c_void = jemalloc_sys::malloc;
#[used]
static _F5: unsafe extern "C" fn(*mut c_void, usize) -> *mut c_void = jemalloc_sys::realloc;
#[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")]
{
unsafe extern "C" {
fn _rjem_je_zone_register();
}

#[used]
static _F7: unsafe extern "C" fn() = _rjem_je_zone_register;
}
}

fn main() {
#[cfg(any(target_os = "linux", target_os = "macos"))]
jemalloc_magic();

let early_dcx = EarlyDiagCtxt::new(ErrorOutputType::default());

// Snapshot a copy of the environment before `rustc` starts messing with it.
Expand Down
Loading