Figure out the performance impacts of making Cell::compartment() take two loads.

RESOLVED WORKSFORME

Status

()

Core
JavaScript Engine
RESOLVED WORKSFORME
6 years ago
5 years ago

People

(Reporter: terrence, Unassigned)

Tracking

Trunk
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Reporter)

Description

6 years ago
One way we could get faster allocator performance is to increase the arena size, thus taking our slow path less frequently.  Normally, this would be a huge increase in fragmentation and memory usage for a small gain.  However, once we have a Nursery -- a second heap -- the balance changes.  If the larger arenas are limited to the Nursery, then this can be a performance win without impacting the size of our long-lived heap.

Unfortunately, |Cell::compartment()| assumes that all GCThings will be in an arena sized area and just looks up the compartment pointer at the front of the current arena.  If we want to store GCThings in variably sized arenas, then we will need to fix this.  The simple way will be to add an extra load to this path and store the arena's compartment pointer somewhere else (like the chunk).

First, we should find out what the cost of the extra lookup will be.  It has been pointed out that making the compartment* for all arenas not alias the same cache line may actually improve performance.  We should find out.  If the performance is slower, we can probably address most of it by passing the compartment to hot-spots manually.

Secondly, we should see if we can get any memory benefits by allocating smaller, or even dynamically sized, arenas, particularly after Compartment-Per-Global lands.  This may be a more elegant solution than adding a specialized heap to the Compartment itself.
(Reporter)

Comment 1

5 years ago
An implementation is in Bug 841059, with some earlier discussion in Bug 706885. The result is in the try run here:

https://tbpl.mozilla.org/?tree=Try&rev=82ef8e5a1d63

The final verdict is: no impact.

There does appears to be a slight regression on kraken on OSX 10.8; however, this is the /only/ test that regressed and there are other data-points nearby that are regressed similarly, so I think it is probably noise.
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → WORKSFORME
You need to log in before you can comment on or make changes to this bug.