Use gecko jemalloc in rust code

RESOLVED FIXED

Status

()

RESOLVED FIXED
4 years ago
2 years ago

People

(Reporter: rillian, Assigned: froydnj)

Tracking

Trunk
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox42 affected)

Details

Attachments

(1 attachment)

Rust has its own jemalloc implementation, which makes it unsafe to pass memory ownership between C++ and rust code in gecko. Even if we avoid that, separate allocators increase overhead and blind about:memory reporting.

This bug is about figuring out how to hook the gecko allocator into rust-side gecko code, either with a source shim, or with custom toolchain builds.

Follow-up from https://bugzilla.mozilla.org/show_bug.cgi?id=1175359#c33
There's a few options of how to tackle this, but unfortunately the "best option" is not available today. Right this red-hot second you can compile recompile Rust to use the system allocator (./configure --disable-jemalloc) and then hope that the system allocator is overwritten to redirect to jemalloc, but I don't think that's available on all platforms.

Long-term what you really want is to implement Rust's allocation API with shims that redirect to Gecko's jemalloc. I've written an RFC on this topic, which will take some time to be accepted (if at all), but I have a sample implementation available as well.

RFC: https://github.com/rust-lang/rfcs/pull/1183
impl: https://github.com/alexcrichton/rust/commits/less-jemalloc

I'll try to keep this updated with the progress on the Rust side of things!
Thanks, Alex.
> Right this red-hot second you can compile recompile Rust to use the system allocator (./configure --disable-jemalloc) and then hope that the system allocator is overwritten to redirect to jemalloc

If the resulting .a/.lib contains relocations for the malloc/free symbols, that would be enough to work in Firefox.

> but I don't think that's available on all platforms.

I'm not sure what you mean here. Do you mean configure --disable-jemalloc is not available on all platforms?
Flags: needinfo?(acrichton)
> If the resulting .a/.lib contains relocations for the malloc/free symbols, that would be enough to work in Firefox.

This kinda happens. On Unix we use posix_memalign, realloc, and free. On Windows we use HeapAlloc, HeapReAlloc, and HeapFree.

> Do you mean configure --disable-jemalloc is not available on all platforms?

Ah no we've just found in the past that overriding the system allocator only worked well for us on Linux, not on Mac or Windows, but if you guys have ways to hook into the malloc/free symbols then that should do the trick!
Flags: needinfo?(acrichton)
(In reply to Alex Crichton [:acrichto] from comment #4)
> > If the resulting .a/.lib contains relocations for the malloc/free symbols, that would be enough to work in Firefox.
> 
> This kinda happens. On Unix we use posix_memalign, realloc, and free.

That works for us.

> On Windows we use HeapAlloc, HeapReAlloc, and HeapFree.

Arg, /that/ doesn't.
 
> > Do you mean configure --disable-jemalloc is not available on all platforms?
> 
> Ah no we've just found in the past that overriding the system allocator only
> worked well for us on Linux, not on Mac or Windows, but if you guys have
> ways to hook into the malloc/free symbols then that should do the trick!

malloc/free are handled differently for each platform, but yes, in the end we have tricks to make this work. However, we don't cover the Heap* functions on Windows, only the malloc/free family.
Ok, we could add a --enable-force-use-malloc or something like that to the configure script which forces Windows to use malloc/free instead of the Heap* functions. I'd prefer to for now pursue the allocator RFC, but if push comes to shove we can add this configuration!
Actually, now that I realize we have Heap* uses in gecko, I'm pondering whether to hook those as well...
rustc --crate-type staticlib will now use the system allocator. Will this pick up our hooks when linked into XUL?

https://github.com/rust-lang/rfcs/pull/1183#issuecomment-126103114
Not quite just yet (the RFC has yet to be implemented), but if the hooks pick up calls to the standard malloc/free functions then it should all work just fine once implemented!
The upstream support for this has now landed in the nightly compiler, so it should be possible to take care of this now with nightly Rust:

    https://github.com/rust-lang/rust/pull/27400
(In reply to Alex Crichton [:acrichto] from comment #10)
> The upstream support for this has now landed in the nightly compiler, so it
> should be possible to take care of this now with nightly Rust:
> 
>     https://github.com/rust-lang/rust/pull/27400

FWIW, the files allocator-override.rs and allocator-dummy.rs in that commit are probably a useful place to start looking at.
Nathan, do you have time to look at this?

https://doc.rust-lang.org/nightly/book/custom-allocators.html#default-allocator (and comment 10) says rustc will default to the system allocator for library output, so it may be this is actually fixed now that we're using rustc 1.4, assuming that ends up calling gecko's jemalloc wrappers.
Flags: needinfo?(nfroyd)
(Assignee)

Comment 13

3 years ago
(In reply to Ralph Giles (:rillian) from comment #12)
> Nathan, do you have time to look at this?
> 
> https://doc.rust-lang.org/nightly/book/custom-allocators.html#default-
> allocator (and comment 10) says rustc will default to the system allocator
> for library output, so it may be this is actually fixed now that we're using
> rustc 1.4, assuming that ends up calling gecko's jemalloc wrappers.

Yes!
Flags: needinfo?(nfroyd)
(Assignee)

Comment 14

3 years ago
OK, so assuming I'm doing everything correctly, it looks like rust 1.4 --crate-type staticlib declares a dependency on alloc_system, which is what we want...at least on unix-ish systems.  alloc_system on Windows uses Windows-specific memory management functions (HeapAlloc et al).  Which I think means, symbol hacking notwithstanding, that we can't do the right thing on Windows yet.

--crate-type staticlib is what we currently use and what I think we're planning on using in the future...?  --crate-type rlib appears to want to use alloc_jemalloc, which doesn't work for us.

1.4 does not support custom allocators, AFAICT.

So I guess 1.4 only gets us halfway or so there, and we have to wait for 1.5 to write our own replacement allocator that uses our jemalloc everywhere, then?
Ah yeah we wanted to avoid the CRT on Windows for allocation as it seemed that the CRT just delegated to the kernel32 functions anyway. Out of curiosity, what do you do in Windows right now for other C libraries? Do you hijack the malloc/free symbols somehow?

Also to clarify, a crate build as an rlib doesn't actually inject an dependency on alloc_jemalloc, it's actually left as unresolved basically. Only "at the last minute", e.g. when building a staticlib, are allocator dependencies resolved. Basically, yes, --crate-type staticlib uses alloc_system by default, but --crate-type rlib just defers that decision until later (e.g. when the rlib is then pulled into a staticlib or an exe).

Unfortunately custom allocators (e.g. writing your own alloc_system or alloc_jemalloc) is an unstable feature right now and requires a nightly compiler, but otherwise it should be possible to do so. I can help out if that's the road you'll want to go down!
(In reply to Alex Crichton [:acrichto] from comment #15)
> Ah yeah we wanted to avoid the CRT on Windows for allocation as it seemed
> that the CRT just delegated to the kernel32 functions anyway. Out of
> curiosity, what do you do in Windows right now for other C libraries? Do you
> hijack the malloc/free symbols somehow?

Essentially, yes. (see comment 5)
Oh right, sorry I forgot about that. In that case you'll likely want to write your own allocator crate to use on all platforms (to satisfy the Windows case). To do that you'll need a compiler which has unstable features enabled (e.g. a standard built-from-source compiler or something from our nightly channel), and then you'll need to write a crate which implements the allocator interface and redirects to the necessary Gecko allocation functions.

The crate itself would look like https://github.com/rust-lang/rust/blob/master/src/test/auxiliary/allocator-dummy.rs and would basically just be a bunch of extern blocks + function calls (not a lot of logic).
Custom allocators RFC is in final call. https://github.com/rust-lang/rfcs/pull/1398
(Assignee)

Comment 19

3 years ago
Created attachment 8738218 [details]
rust allocator module to integrate with gecko

Attached is a Rust module to enable Rust code in Gecko to use Gecko's jemalloc rather than the allocator in the standard library.  It requires nightly Rust (and AFAICT, that limitation is not going away particularly soon).  I haven't tried integrating it into the Gecko build alongside bug 1163224; declaring |extern crate alloc_gecko;| and compiling simple programs does seem to work and I can confirm that my allocator is the one being compiled in, so that's a good sign.
(Assignee)

Updated

3 years ago
Assignee: nobody → nfroyd
Does it require all rust crates to import the allocator crate or just our meta crate? If the former, that's not going to be nice...
(Assignee)

Comment 21

3 years ago
(In reply to Mike Hommey [:glandium] from comment #20)
> Does it require all rust crates to import the allocator crate or just our
> meta crate? If the former, that's not going to be nice...

I assume the meta crate only, though I haven't tried it.  I can't imagine having to declare it in individual crates; that seems like a nightmare and would also cause issues with interoperability...
Yes, only the meta crate needs to specify the allocator here, all other crates will then end up using that one.
Depends on: 1280578
froydnj, is this done now?
Flags: needinfo?(nfroyd)
(Assignee)

Comment 24

2 years ago
(In reply to Nicholas Nethercote [:njn] from comment #23)
> froydnj, is this done now?

I put a patch up in bug 1254779 to provide tests for allocation function hooking.  The tests pass on try, so regardless of whether the tests get committed or not, I think we can say this is done.
Flags: needinfo?(nfroyd)
Hooray.
Status: NEW → RESOLVED
Last Resolved: 2 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.