1241352 - Increase FallbackICStubSpace chunk size to reduce heap churn

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Nicholas Nethercote [:njn]]

I did some cumulative profiling with DMD and found that the chunk size of
FallbackICStubSpace's allocator (256 bytes) is smaller than ideal, causing more
calls to malloc/free than necessary.

I tried changing it to 1024, 4096 and 8192, and found that 4096 was the best.
Summary stats:

-  256: Total: 4,227,855,576 bytes in 2,110,196 blocks
- 1024: Total: 4,240,921,416 bytes in 2,071,114 blocks
- 4096: Total: 4,184,653,168 bytes in 2,043,207 blocks
- 8192: Total: 4,737,642,408 bytes in 2,096,809 blocks

Choosing 4096 bytes minimizes the number of calls to malloc, reducing it by
over 3% compared to 256 bytes. 4096 also reduces the cumulative size of those
mallocs, which was an unexpected bonus; I think this must be due to reduced
fragmentation. And it's also the same chunk size used by OptimizedICStubSpace.

(Note: the 8192 numbers are a bit on the high side, possibly due to natural
variation between runs. But when I dug into the numbers it was clear that 8192
was worse than 4096 because lots of the time these allocators allocate less
than 4096 bytes.)

Comment 1

[Nicholas Nethercote [:njn]]

Attachment: Increase FallbackICStubSpace chunk size to reduce heap churn

Comment 2

[Jan de Mooij [:jandem]]

Comment on attachment 8710202 [details] [diff] [review]
Increase FallbackICStubSpace chunk size to reduce heap churn

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

(In reply to Nicholas Nethercote [:njn] from comment #0)
> And it's also the same chunk size used by OptimizedICStubSpace.

Each BaselineScript has its own FallbackICStubSpace, but there's only one OptimizedICStubSpace per Zone. That's why FallbackICStubSpace's chunk size was so much smaller. You're the expert on this and if measurements show 4096 doesn't actually waste a lot more memory, it's fine to change it probably. Avoiding malloc is also nice for performance.

Maybe we should have a single FallbackICStubSpace per Zone or compartment, but then we could only free its pages if the Zone/compartment has no Baseline scripts on the stack (Baseline scripts on the stack can't be discarded). Might be a reasonable trade-off..

Comment 3

[Jan de Mooij [:jandem]]

(In reply to Jan de Mooij [:jandem] from comment #2)
> Maybe we should have a single FallbackICStubSpace per Zone or compartment,
> but then we could only free its pages if the Zone/compartment has no
> Baseline scripts on the stack (Baseline scripts on the stack can't be
> discarded). Might be a reasonable trade-off..

Thinking about this more, worst case we could look at the scripts on the stack and mark any BumpChunks they're using, and free the other chunks.

Let's land this patch first though. I'll do some measurements later and file a follow-up bug to see if we can move this allocator to the Zone.

Comment 4

[Jan de Mooij [:jandem]]

I did some measurements in the browser and it's extremely uncommon (I got no hits at all) to have active Baseline scripts on the stack when we discard JIT code, so a per-Zone allocator makes sense.

The only case I'm worried about is when scripts can run uninterrupted for a long time, for instance workers or the shell. In that case, a single Baseline script on the stack could prevent us from freeing any stub memory.

Ideally we'd have something like a LifoAlloc with reference counted chunks. BaselineScripts could hold references to these chunks. Basically I want ExecutableAllocator but without the Executable part. I think it should be possible to move the pool related code out of ExecutableAllocator and into a new class, so we can reuse it here.

Comment 5

[Nicholas Nethercote [:njn]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/406f2af156e70f008242c91eade94c50c7e8fdb3
Bug 1241352 - Increase FallbackICStubSpace chunk size to reduce heap churn. r=jandem.

Comment 6

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/406f2af156e7