1827918 - Tidy GC allocation paths in preparation for adding a nursery fast path

Status: RESOLVED FIXED

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

Description

[Jon Coppeard (:jonco)]

The GC allocation code is quite complicated. I wrote some patches to tidy it up and make it more consistent between the different allocation kinds.

Comment 1

[Jon Coppeard (:jonco)]

Attachment: Bug 1827918 - Part 1: Don't call TenuredCell consturctor but rely on derived constructor r?sfink

Currently we call both the TenuredCell consturctor and the derived constructor
for tenured allocations. While this is not illegal, it's strange and we should
rely on the derived constructor.

The patch changes the tenured allocation paths to use void* pointers to
unconstructed memory.

I don't know why the change to cell header initialization was necessary. It
seems the TenuredCell constructor initialized the header to zero for some
reason, but I don't see why it should have.

Comment 2

[Jon Coppeard (:jonco)]

Attachment: Bug 1827918 - Part 2: Move dynamic slot allocation out of GC allocation path r?jandem

This moves the dynamic slot allocation code that is currently duplicated
between nursery and tenured allocation paths into NativeObject instead.

Depends on D175372

Comment 3

[Jon Coppeard (:jonco)]

Attachment: Bug 1827918 - Part 3: Common up object construction and call the derived constructor r?sfink

Previously we didn't call any constructor for nursery-allocated objects, and we
always called the JSObject constructor for tenured objects. This makes object
constructor more like the other paths and calls the constructor in one place at
the end.

Depends on D175373

Comment 4

[Jon Coppeard (:jonco)]

Attachment: Bug 1827918 - Part 4: Refactor BigInt allocation along the same lines as the other kinds r?sfink

Depends on D175374

Comment 5

[Jon Coppeard (:jonco)]

Attachment: Bug 1827918 - Part 5: Rename AllocateString's type parameter in line with the other methods r?sfink

Depends on D175376

Comment 6

[Jon Coppeard (:jonco)]

Attachment: Bug 1827918 - Part 6: Forward declare CellAllocator in Cell.h since derived types already need to include this r?sfink

This is just so you don't have to include both Cell.h and Allocator.h to
declare a derived cell type.

Depends on D175378

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e8cfac05da4a
Part 1: Don't call TenuredCell consturctor but rely on derived constructor r=sfink
https://hg.mozilla.org/integration/autoland/rev/cd8376894002
Part 2: Move dynamic slot allocation out of GC allocation path r=jandem
https://hg.mozilla.org/integration/autoland/rev/684d845a4a42
Part 3: Common up object construction and call the derived constructor r=sfink
https://hg.mozilla.org/integration/autoland/rev/00cf9f71c60e
Part 4: Refactor BigInt allocation along the same lines as the other kinds r=sfink
https://hg.mozilla.org/integration/autoland/rev/e8e565d555fa
Part 5: Rename AllocateString's type parameter in line with the other methods r=sfink
https://hg.mozilla.org/integration/autoland/rev/34ed400b7bb1
Part 6: Forward declare CellAllocator in Cell.h since derived types already need to include this r=sfink
https://hg.mozilla.org/integration/autoland/rev/179b8c4df1ad
apply code formatting via Lando

Comment 8

[Sandor Molnar]

https://hg.mozilla.org/mozilla-central/rev/e8cfac05da4a
https://hg.mozilla.org/mozilla-central/rev/cd8376894002
https://hg.mozilla.org/mozilla-central/rev/684d845a4a42
https://hg.mozilla.org/mozilla-central/rev/00cf9f71c60e
https://hg.mozilla.org/mozilla-central/rev/e8e565d555fa
https://hg.mozilla.org/mozilla-central/rev/34ed400b7bb1
https://hg.mozilla.org/mozilla-central/rev/179b8c4df1ad