Closed
Bug 1121235
Opened 11 years ago
Closed 5 years ago
Consider allocating GC chunks on the malloc heap
Categories
(Core :: JavaScript: GC, defect)
Tracking
()
RESOLVED
WONTFIX
People
(Reporter: n.nethercote, Unassigned)
Details
Attachments
(1 file)
|
8.88 KB,
patch
|
Details | Diff | Splinter Review |
GC chunks are currently allocated with mmap/VirtualAlloc. There are two problems with this.
- There's lots of tricky code to get the alignment right in a way that doesn't waste address space. AIUI this code more or less duplicates similar code in jemalloc.
- Heap profilers like DMD don't see GC chunks, which are a big fraction of memory usage.
It might be feasible to allocate GC chunks on the malloc heap. The heap allocation functions that provide aligned allocations vary between platforms, but I think this would suffice for allocation:
> #if defined(MOZ_MEMORY)
> use posix_memalign
>
> #elif defined(HAVE_POSIX_MEMALIGN)
> use posix_memalign, and on Mac do the non-aligned workaround seen in
> moz_posix_memalign (we can't use moz_posix_align directly from
> Spidermonkey, alas)
>
> #elif defined(HAVE_MEMALIGN)
> use memalign
>
> #elif WINDOWS
> use _aligned_alloc
>
> #else
> #error
and for deallocation:
> #if defined(MOZ_MEMORY) || defined(HAVE_POSIX_MEMALIGN) || defined(HAVE_MEMALIGN)
> use free
>
> #elif WINDOWS
> use _aligned_free
>
> #else
> #error
I based this off VolatileBuffer, which already does aligned allocations, so it seems that every configuration we care about has a suitable aligned allocation function. The MOZ_MEMORY one will always be used when jemalloc is in use, which is the normal case.
Some other things we'd need to check:
- That this wouldn't increase physical or virtual memory usage.
- Parts of GC chunks get decommitted. Is this a problem?
| Reporter | ||
Comment 1•11 years ago
|
||
Here's a proof of concept. It just assumes that posix_memalign is available,
which is fine on normal Linux builds. I did a try run that looks fine, so
there's no obvious problems:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=a0b175874a0d
| Reporter | ||
Updated•11 years ago
|
Assignee: nobody → n.nethercote
Status: NEW → ASSIGNED
| Reporter | ||
Comment 2•11 years ago
|
||
Emanuel, is this something you'd be interested in looking at? It seems right up your alley :)
Flags: needinfo?(emanuel.hoogeveen)
Comment 3•11 years ago
|
||
(In reply to Nicholas Nethercote [:njn] from comment #0)
> - There's lots of tricky code to get the alignment right in a way that
> doesn't waste address space. AIUI this code more or less duplicates similar
> code in jemalloc.
jemalloc's code here is more simplistic, in that it doesn't try to align unaligned chunks and doesn't have a last ditch pass to skip unalignable chunks. A version of the patch in bug 1005844 may eventually land for jemalloc3, but jemalloc's maintainer didn't seem enamored with the idea half a year ago, and hasn't responded to anything I've said since.
Our fork of jemalloc2 *does* currently cache a number of chunks for later use rather than immediately freeing them. This would be nice to have for the GC to reduce the number of system calls in some situations, although we've talked about implementing something more dynamic based on the allocation rate. However, this code has not been upstreamed to jemalloc3, so when we switch it will go away. I can port the code over to jemalloc3 pretty easily (build system aside), but for now I don't know if and when it will be coming back.
> - Heap profilers like DMD don't see GC chunks, which are a big fraction of
> memory usage.
How hard would this be to add? All GC allocations do go through the same choke point.
> Some other things we'd need to check:
> - That this wouldn't increase physical or virtual memory usage.
I don't think it would increase physical memory usage, so long as we ensure that jemalloc and the GC use the same chunk sizes on all platforms (I think this is currently true). I guess it would add a tiny bit of bookkeeping for jemalloc, but we're talking at most a few thousand allocations being tracked.
As said above, jemalloc might not do as well at preventing or coping with fragmentation, which could make us run out of virtual memory more quickly.
> - Parts of GC chunks get decommitted. Is this a problem?
I don't think so, assuming there's no security flags preventing us from decommitting the memory.
Comment 4•11 years ago
|
||
By the way, I recently simplified this logic to match the latest patch in bug 1005844: bug 1118481. That doesn't affect the other points, but at least the logic is a lot less tricky now :)
Flags: needinfo?(emanuel.hoogeveen)
Comment 5•11 years ago
|
||
> As said above, jemalloc might not do as well at preventing or coping
> with fragmentation, which could make us run out of virtual memory
> more quickly.
I'd argue that the use of mmap for GC chunks actually contributes to the fragmentation, since we end up with GC chunks and jemalloc chunks intertwined. Removing that will certainly help jemalloc make better use of the address space, although there are other things that use mmap.
> I can port the code over to jemalloc3 pretty easily
I have that mostly covered already.
Comment 6•11 years ago
|
||
(In reply to Mike Hommey [:glandium] from comment #5)
> I'd argue that the use of mmap for GC chunks actually contributes to the
> fragmentation, since we end up with GC chunks and jemalloc chunks
> intertwined. Removing that will certainly help jemalloc make better use of
> the address space, although there are other things that use mmap.
It will certainly make the chunk recycling less effective at coalescing address space - but jemalloc and the GC use the same chunksize and alignment, so I don't think it'll make things worse otherwise. If the GC allocator succeeds at allocating a chunk where jemalloc did not attempt to align, that also plugs a hole in the address space to give jemalloc a better chance of finding already-aligned chunks.
> I have that mostly covered already.
That's good to hear!
Comment 7•11 years ago
|
||
FWIW I do support what this bug is suggesting, I'm just not sure we're ready for it quite yet. The existing jsapi-test might be able to stay if we can make jemalloc work the same way (so long as we keep using mmap/VirtualAlloc directly in the jsapi-test itself).
| Reporter | ||
Updated•10 years ago
|
Assignee: n.nethercote → nobody
Status: ASSIGNED → NEW
| Reporter | ||
Updated•5 years ago
|
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → WONTFIX
You need to log in
before you can comment on or make changes to this bug.
Description
•