Repository navigation
liballoc_system without #[global_allocator] uses jemalloc #45966
Description
Activity
- addedC-enhancementCategory: An issue proposing an enhancement or a PR with one.Category: An issue proposing an enhancement or a PR with one.
on Nov 14, 2017 @alexcrichton wrote #46117 (comment)
Unfortunately I think it's basically just impossible to fix
System.alloccalling jemalloc because of linker trickery. Our only recourse is to remove jemalloc.Do you mean impossible while also making LLVM use jemalloc? If we revert (relevant parts of) #31460 and go back to always compiling jemalloc with
--with-prefix=something, wouldn’t that fix this issue? What’s the plan for LLVM once thealloc_jemalloccrate is removed?Prefixing jemalloc symbols would resolve this issue, but I would argue that would be a net negative. It's not great to have two competing allocators both running at the same time in a single process.
As long as alloc_jemalloc/jemallocator/whatever is configured to have non-prefixed symbols, LLVM will continue to use it AFAIK.
Reacted by Sander Maijers@SimonSapin ah yeah sorry what I meant was we could indeed prefix the symbols but the intention is to get LLVM to use jemalloc (as it makes it ~10% faster historically). I think our long term plan is to remove jemalloc from libtsd but leave it in the compiler, so rustc itself will still use jemalloc but Rust programs by default will not.
If you really want to talk to specifically glibc malloc, you can link to
__libc_malloc, but that's probably not a portable thing.@alexcrichton Right, I think that split between rustc and std is probably the best eventual outcome.
@sfackler Interesting. I think we still want
alloc_systemto call plainmalloc, though. For example Firefox redefinesmalloc(in a fork of an old version of jemalloc) and expects Rust dynamic libraries to use it.It's not great to have two competing allocators both running at the same time in a single process.
I can see that point. But then we should not be providing APIs that pretend to do this, while they actually do not.
TL;DR: let’s switch rustc to unprefixed jemallocator and restore symbol prefixes in alloc_jemalloc now?
So, we’re discussing a number of desirable but apparently competing points. I think we can have our cake and eat it too. I’m gonna name them to untangle everything without repeating lengthy phrases over and over.
- A. Stable users can choose to have
std::heap::Heapin Rust executables use jemalloc - B. Stable users can choose to have
std::heap::Heapin Rust executables use the system allocator - D. Rustc and LLVM-in-rustc use jemalloc (for that 10% perf improvement)
- E. C/C++ libraries that uses
malloclinked with a Rust program end up using the same allocator asstd::heap::Heap - F.
std::heap::Systemuses the system allocator even if not selected forstd::heap::Heapwith#[global_allocator]: fixing this bug
Currently we have D and E on some platforms since
alloc_jemallocconfigures jemalloc without a symbol prefix, and A since it’s the default.There’s a number of changes we can make. These are not blocked as far as I can tell:
- 1. Make
alloc_jemallocalways configure jemalloc with a symbol prefix. - 2. Remove
alloc_jemallocand make B the default - 3. Add an
unprefixedCargo feature to https://crates.io/crates/jemalloc-sys (and perhaps forward it through https://crates.io/crates/jemallocator) so that it configures jemalloc without a symbol prefix. (The default would still be--with-prefix=_rjem_.) - 4. Make rustc use
jemallocatorwith 3 instead ofalloc_jemalloc
This is blocked on API design decisions:
- 5. Stabilize
#[gloabal_allocator]Tracking issue for changing the global, default allocator (RFC 1974) #27389 andAllocAllocator traits and std::heap #32838.
With 1 alone we gain F, but lose D and E.
With 2 alone we gain B and F, but lose A, D, and E. Which of A or B is more desirable (if we have to choose) is debatable. Adding 5 restores A.
#33082 (comment) suggests that doing 2 is planned, but not until until 5 is solved (presumably to avoid losing A). At that point we’ll likely also want to avoid losing D (or E). Doing 3 and 4 seems to be to be the easiest way to achieve that.
However we don’t need to wait for 5 to be stabilized before doing 3 or 4. So I suggest doing 3, 4, then 1 now-ish. Both in order to fix F, and to be ready to do 2 later when 5 is unblocked. The only point we would temporarily lose is E for stable users.
@alexcrichton, @sfackler, what do you think?
- A. Stable users can choose to have
Seems plausible to me!
I'm remembering now though that this probably won't work unfortunately. The standard library is created as a dynamic library which currently fixes the allocator to jemalloc (
alloc_jemallocthat is). That will be required to get fixed first before we can have alloc_jemalloc and jemallocatorAny mention of "use jemallocator" above implies selecting it with
#[global_allocator]. My understanding is thatalloc_jemallocand its copy of jemalloc are only linked when#[global_allocator]is not used. Otherwise jemallocator could never be used at all.Er yes I think I understand what you're advocating for, and it sounds like a great plan. What I mean is that if you do it you'll get a compile time error and it will fail to compile.
I don’t understand what error that would be.
Er yes that's what I mean by "fixing the allocator" and "required to get fixed", it's a bit obscure... Turns out it's not a compile time error (but it should be right now) but you can observe the behavior with:
#![feature(global_allocator, allocator_api)] use std::heap::*; #[global_allocator] static A: B = B; struct B; static mut HIT: bool = false; unsafe impl<'a> Alloc for &'a B { unsafe fn alloc(&mut self, layout: Layout) -> Result<*mut u8, AllocErr> { HIT = true; System.alloc(layout) } unsafe fn dealloc(&mut self, ptr: *mut u8, layout: Layout) { HIT = true; System.dealloc(ptr, layout) } } fn main() { println!("hello!"); assert!(unsafe { HIT }); }
That program "accidentally" succeeds on Linux due to how the dynamic linker works but it fails on Windows where the dynamic linker works differently.
Er, when compiling that program with
-C prefer-dynamic, that is, like how the compiler is compiled.This is a pretty old issue now and I think is something we're probably not going to change to preserve performance of rustc, so I'm going to close this in favor of #36963 where we'll intentionally be routing
mallocthrough jemalloc in the compiler specifically because we want LLVM to use jemalloc@alexcrichton Just to be clear, it would still be wrong if outside the compiler, calling
System::allocwould invoke jemmalloc, right? You just consider fixing that behavior part of #36963 now?@RalfJung it's not really wrong per se in the sense that this is how jemalloc/Linux are designed. The "system allocator" on Linux isn't glibc malloc but rather the
malloc/freesymbols. It just so happens that jemalloc's build by default overrides those symbols.This is also intentional behavior that we desired on OSX/Linux because we want LLVM's usage of
mallocandfreeto get routed to one global allocator, jemalloc, which is shared with rustcIt’s not wrong for
System::allocto use jemalloc is configured to hook itself into the system allocator. However it’s definitely unexpected IMO that Rust does so by default. #55238 should fix this.
This was found by @RalfJung in #45955.
alloc_system::System::alloccallslibc::mallocwhich is defined as:When
#[global_allocator]is not used, the current default for executables isalloc_jemalloc, which links jemalloc as configured insrc/liballoc_jemalloc/build.rs. On most platforms, this is without--with-jemalloc-prefix, which causes jemalloc to define an unprefixedmallocsymbol that "overrides" libc’s and ends up being used byalloc_system.So
alloc_systemdoesn’t do what the name suggests, in this situation.Like https://github.com/alexcrichton/jemallocator/issues/19 this problem will disappear when
alloc_jemallocis eventually removed, but in the meantimealloc_systemdoesn’t always do what the name says it does.We stopped prefixing jemalloc symbols in #31460 in order to make LLVM use them. Could we perhaps only do this when compiling a compiler? Or perhaps the compiler could switch to using the
jemallocatorcrate, which would gain a Cargo feature flag to disable prefixing? It would do something like that anyway to keep that LLVM+jemalloc benefit whenalloc_jemallocis removed and the default for executables is changed toalloc_system.CC @alexcrichton