1473539 - Investigate using mapped pages rather than malloc in LIFO allocator

Status: RESOLVED INVALID

(Core :: JavaScript: GC, enhancement)

Description

[Jon Coppeard (:jonco)]

LifoAlloc always allocates memory in power-of-two sized blocks that are at least one page long.  We should be able to make it map pages directly and avoid malloc.  This should result in better performance in a multithreaded situation as malloc use a lock to protect its internal data structures.

Comment 1

[Jon Coppeard (:jonco)]

Attachment: bug1473539-lifo-tidy

First a patch to do some preparation and tidyup:

 - renamed LifoAlloc::BumpChunk to UniqueBumpChunk for clarity
 - added a delete policy to use of UniquePtr for use in next patch
 - attempt to calculate BumpChunk reserved space rather than hardcode and assert

For the latter I had to remove the power of two assertion in ComputeByteAlignment because it seems we can't MOZ_ASSERT in a constexpr function.  This is still correct if called with non-power-of-two arguments, it's just not the expected use.  Up to you whether you think this is worth it.

Comment 2

[Jon Coppeard (:jonco)]

Attachment: bug1473539-map-pages

Patch to map LIFO alloc'd pages directly rather than using malloc.  I had to change the exception handler code so that the ProtectedRegionTree was allocated after initialization as it uses a LifoAlloc.

Comment 3

[Nicolas B. Pierron [:nbp]]

Comment on attachment 8990055 [details] [diff] [review]
bug1473539-map-pages

Review of attachment 8990055 [details] [diff] [review]:
-----------------------------------------------------------------

This patch looks great, still I am worried about the potentially high number of syscalls and I suspect Bug 1437600 might hide potential performance improvement/regression.
Can you submit talos pushes with before and after this patch with the instrumentation from Bug 1437600 disabled in both.

You can disable Bug 1437600 instrumentation by removing this line: https://searchfox.org/mozilla-central/rev/6ef785903fee6c0b16a1eab79d722373d940fd78/js/src/ds/LifoAlloc.h#229

Comment 4

[Jon Coppeard (:jonco)]

(In reply to Nicolas B. Pierron [:nbp] {backlog: 39} from comment #3)
> Can you submit talos pushes with before and after this patch with the
> instrumentation from Bug 1437600 disabled in both.

Good idea.  Initial results show some small improvements on windows and linux and some regressions on OSX:

https://treeherder.mozilla.org/perf.html#/compare?originalProject=try&originalRevision=c6127fa6fbc5&newProject=try&newRevision=58f048e95b8682f448283e0707b073132942f020&framework=1&showOnlyConfident=1

Comment 5

[Jon Coppeard (:jonco)]

(In reply to Jon Coppeard (:jonco) from comment #4)
Further microbenchmarking shows this is significantly slower.  I guess jemalloc does a decent job of recycling the pages rather than allocating them from / returning them to the OS every time.

I'm going to land the cleanup patch in a separate bug and close this.