Closed Bug 427107 Opened 12 years ago Closed 4 years ago

Build with jemalloc on all platforms

Categories

(Core :: Memory Allocator, defect)

defect
Not set

Tracking

()

RESOLVED WORKSFORME

People

(Reporter: benjamin, Assigned: jasone)

References

(Blocks 1 open bug)

Details

In order to implement no-fail memory allocation, OOM callbacks, and (in the future) GC, we want to build with jemalloc everywhere, all of the time.

This certainly means that we can't use the "patched MS CRT" approach we currently use on Windows (because of licensing). So we need jemalloc to be a separately named library with different function names.

This implies, I think, that we need to rewrite current uses of malloc/free to a different name:

option 1) put jemalloc into NSPR and use the PR_Malloc name
option 2) make jemalloc a separate library and use je_malloc or moz_malloc... teach NSPR to use this other library
option 3) make jemalloc a separate library: teach NSPR how to use a pluggable allocator

This also means that instances of strdup and perhaps other CRT functions which forward to malloc will need to be reimplemented in terms of jemalloc instead.
Blocks: 427099
Blocks: 427109
I think we want to continue building jemalloc as part of the CRT on Windows.  May need to see what our options are there.
I think we should continue building the CRT on Windows for our official builds, just because it's smaller and saves us the manifest headaches, but that's not really related.
The problem with moving jemalloc out of the CRT is that it opens up problems with mixed allocator usage.  Many of the standard C functions like strdup() then have to be very carefully used, and I'm guessing that in many places we would have to copy memory out of malloc'ed memory into jemalloc'ed memory, in order to allow other portions of the mozilla code to assume that all memory comes from a single allocator.

Mixed allocator use is really tough to get right.
It's particularly tricky when the library function may or may not call malloc() internally.  (Though if it isn't obvious (like strdup), then we shouldn't be free()ing anything returned.)  Since many libraries will call malloc()/etc, you will end up with a somewhat mixed memory map no matter what.  

As you say, for those like strdup that return an object that the library expects us to call free() on, we may need to wrap the function and copy it out into our own allocator, OR be very very careful how we use the return.  Or we can override malloc/etc, but that has it's own issues.
Assignee: jasone → nobody
Component: General → jemalloc
QA Contact: general → jemalloc
Assignee: nobody → jasone
Depends on: 414946
jason: on windows allocator issues exist anyway, because of the way things work, each dll can have its own heap, and if it's compiled against a different libc, it uses that dll's heap.

windows devs are supposed to know how not to mix up allocators (windows has a number of system allocators which are incompatible too).

for linux, we've had this problem occasionally when an xinputmethod links in libc5 or something insane.

i don't think we should worry about mixed memory. I'm willing to fight those problems, although the suggestion from bug 458942 comment 0 of having a way to recognize if a bit is from jemalloc space or not would be nice (and yes, I'm aware of the cpu cost, i'd be fine with a way to drop in a jemalloc debug version for this purpose).
Blocks: 586962
glandium: is this bug ever going to happen as comment 0 describes? ("we want to build with jemalloc everywhere, all of the time.")
Flags: needinfo?(mh+mozilla)
In practice, we already do build with jemalloc everywhere, all of the time. Except on some tier-3 platforms, on x86 mac (but even that is just inherited from the past, and I think we could enable jemalloc on x86 mac without any trouble nowadays), and for some things where we do need to disable it (asan/fsan/valgrind, although for valgrind, we could still enable it)
Flags: needinfo?(mh+mozilla)
cf. comment 7.
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → WORKSFORME
You need to log in before you can comment on or make changes to this bug.