1993212 - Investigate allocating WeakMap memory with the buffer allocator

Status: NEW

(Core :: JavaScript: GC, task, P2)

Description

[Jon Coppeard (:jonco)]

This should make things more efficient.

Comment 1

[Jon Coppeard (:jonco)]

Attachment: Bug 1993212 - Part 1: Rename reportAllocationOverflow methods to reportAllocOverflow r?jandem

According to mfbt/AllocPolicy.h, allocation policies are supposed to implement
a 'reportAllocOverflow' method. This is used both by the implmementation of the
alloc policy itself and also by containers such as mfbt/HashTable.h when
growing its table.

Our MallocProvider mixin class implements most of the AllocPolicy methods for
a client class, but requires clients to provide a 'reportAllocationOverflow'
method.

Thus clients must provide both methods if they want to be used by containers.
Currently only TrackedAllocPolicy (a.k.a. ZoneAllocPolicy) does this, by having
reportAllocOverflow call reportAllocationOverflow.

The current situation is confusing. We can simplify this by making
MallocProvider use the reportAllocOverflow name and renaming to this
everywhere.

Comment 2

[Jon Coppeard (:jonco)]

Attachment: Bug 1993212 - Part 2: Refactor JS alloc policy header to provide a base class for alloc policies r?jandem

This refactoring gives us a place to put default method definitions without
adding them to every policy.

The patch also renames some alloc policy classes to make it clear they use malloc allocation.

Comment 3

[Jon Coppeard (:jonco)]

Attachment: Bug 1993212 - Part 3: Wait for background GC activity to finish if buffer alloc fails r?jandem

This makes handling OOM simpler in the next patch because we don't need to
retry at a higher level.

Comment 4

[Jon Coppeard (:jonco)]

Attachment: Bug 1993212 - Part 4: Add a way to trace memory allocations owned by standard containers r=jandem!,glandium!

To support using GC memory in containers we need a way to trace that memory.
This patch adds a 'getExternalAllocPtr' method to vector and hash table containers
that can be used to get the pointer to trace.

Comment 5

[Jon Coppeard (:jonco)]

Attachment: Bug 1993212 - Part 5: Add BufferAllocPolicy to allow containers to allocate their memory with the buffer allocator r?jandem

The patch adds BufferAllocPolicy which allocates memory using the buffer
allocator and provides a traceExternalAlloc method to trace the allocations, as
well as updating its knowledge of whether the owning cell is tenured.

Comment 6

[Jon Coppeard (:jonco)]

Attachment: Bug 1993212 - Part 6: Allow multi-threaded buffer allocator use while holding a lock r?jandem

Previously calls to the buffer allocator to allocate/free memory were always
done from the main thread and so no locking was required. GC sweeping can be
split between more than one helper thread so we need a way to make this thread
safe.

The patch adds a way to set up debug build assertions in the buffer allocator
by passing a mutex that the caller must hold. The caller is responsible for
actually taking/releasing the lock.

Comment 7

[Jon Coppeard (:jonco)]

Attachment: Bug 1993212 - Part 7: Take lock when resizing weak map hash table r?jandem

This sets up multi-threaded use of the buffer allocator during sweep slices
using an existing mutex. While sweeping weak maps the mutex is only held when
destroying the weak map iterator as this is the part that may resize the table
and hence call into the allocator. This is also the part required by existing
use of the lock for weak map sweeping.

Comment 8

[Jon Coppeard (:jonco)]

Attachment: Bug 1993212 - Part 8: Rename store buffer lock to sweeping lock r?jandem

Since this mutex is now used for more than synchronizing access to the store
buffer it is renamed to the sweeping lock.

Comment 9

[Jon Coppeard (:jonco)]

Attachment: Bug 1993212 - Part 9: Change WeakMap constructor to either take zone or context and owning JS object r?jandem

Currently the memOf parameter is optional for both constructors. This patch
changes it so that for a weak map with an owning JSObject you need to pass
context and owning object, and for non-owned ones you only pass the zone.

Comment 10

[Jon Coppeard (:jonco)]

Attachment: Bug 1993212 - Part 10: Use the buffer allocator for JS WeakMap/Set objects r?jandem

Finally we can switch over to using the buffer allocator for weak maps, both
the WeakMap allocation and the hash table memory via BufferAllocPolicy.

WeakMap objects remain tenured. I haven't thought about whether it's worth
trying to nursery allocate these.