Skip to content
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.

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

jemalloc does not seem to be working for rustc on macos #82423

Closed
ehuss opened this issue Feb 22, 2021 · 8 comments · Fixed by #82642
Closed

jemalloc does not seem to be working for rustc on macos #82423

ehuss opened this issue Feb 22, 2021 · 8 comments · Fixed by #82642
Labels
C-bug Category: This is a bug.

Comments

@ehuss
Copy link
Contributor

ehuss commented Feb 22, 2021

jemalloc is supposed to be enabled on macos for rustc, but with all the testing I've done, it does not appear to work.

Noted by Thom Chiovoloni on Zulip here, the jemalloc-sys crate prevents using unprefixed symbols here, which IIUC, means that it is building with symbols like __rjem_malloc, which AFAIK are never actually used.

If it indeed does not work, then I think it would be good to consider either looking into fixing it (which I suspect may not be easy), or stop building it on CI.

Tested with most recent nightly (rustc 1.52.0-nightly (3e826bb11 2021-02-21)) and master (a15f484).

@ehuss ehuss added the C-bug Category: This is a bug. label Feb 22, 2021
@sfackler
Copy link
Member

You can't override the macos malloc by shadowing its symbols (I think very early allocations in the loader still use the original malloc or something?). Instead, jemalloc actually hooks directly into some macos allocator APIs to replace the guts behind the original symbols IIRC.

@sfackler
Copy link
Member

@ehuss
Copy link
Contributor Author

ehuss commented Feb 23, 2021

Thanks @sfackler for taking a look! Do you happen to know if there's some way to validate that it is working?

When I run rustc with the environment variable _RJEM_MALLOC_CONF=stats_print:true I get that rustc has allocated 36224 bytes regardless of how big of a crate I build. Is there maybe a better way to determine if it is actually working?

@sfackler
Copy link
Member

Ah - when statically linking to jemalloc, the zone_register constructor function appears to be dropped by the linker: jemalloc/jemalloc#708.

The compiler already manually marks other bits of jemalloc as #[used]: https://github.com/rust-lang/rust/blob/master/compiler/rustc/src/main.rs#L10-L27.

Extending that with

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

extern "C" {
    fn _rjem_je_zone_register();
}

Gets it registered properly for me with a little test crate.

@ehuss
Copy link
Contributor Author

ehuss commented Feb 23, 2021

Ah, I can confirm with those changes it seems to be working.

I might do some benchmarking to see how it does, but pending #81782, it may not be necessary.

@ehuss
Copy link
Contributor Author

ehuss commented Feb 23, 2021

I ran a few benchmarks, and it looks promising:

Benchmark #1: a.sh — system malloc
  Time (mean ± σ):     19.119 s ±  0.058 s    [User: 89.221 s, System: 6.249 s]
  Range (min … max):   19.070 s … 19.183 s    3 runs

Benchmark #2: b.sh — jemalloc + patch
  Time (mean ± σ):     17.462 s ±  0.030 s    [User: 80.078 s, System: 5.993 s]
  Range (min … max):   17.441 s … 17.496 s    3 runs

Benchmark #3: c.sh — nightly-2021-02-22
  Time (mean ± σ):     22.273 s ±  0.053 s    [User: 105.825 s, System: 5.951 s]
  Range (min … max):   22.211 s … 22.305 s    3 runs

Summary
  'b.sh' ran
    1.09 ± 0.00 times faster than 'a.sh'
    1.28 ± 0.00 times faster than 'c.sh'

(Not sure why the prebuilt nightly is so much slower. Maybe some random setting like MACOSX_DEPLOYMENT_TARGET?)

These are pretty typical numbers of the various projects I tried.

I also tested tikv-jemalloc-sys, but I didn't see much difference from the current jemalloc.

@jq-rs
Copy link

jq-rs commented Feb 23, 2021

Pretty neat! mimalloc would need macOS support too, so this should give clues on how to add it.

@thomcc
Copy link
Member

thomcc commented Feb 24, 2021

You should make sure this works on macOS pre-10.12, as this version changed how a number of things work (without looking at jemalloc, possibly most relevant being interpose)

Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this issue Mar 2, 2021
JohnTitor added a commit to JohnTitor/rust that referenced this issue Mar 5, 2021
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this issue Mar 8, 2021
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this issue Mar 8, 2021
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this issue Mar 8, 2021
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this issue Mar 8, 2021
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this issue Mar 8, 2021
@bors bors closed this as completed in 6e52b23 Mar 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: This is a bug.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants