1725348 - Rewrite array allocation code to be based on the per-global array shape

Status: RESOLVED FIXED

(Core :: JavaScript Engine, task, P3)

Description

[Jan de Mooij [:jandem]]

The code in NewArray is optimized using the NewObjectCache. This cache is brittle outside of micro-benchmarks (we'd like to remove it eventually), and there's a lot of complexity, for example to handle defining the length-property.

We can now rewrite this code to be based on a shape that gets passed in. In most cases this will be the array shape cached in the global object. This gives us more predictable performance and is also a lot simpler.

Comment 1

[Jan de Mooij [:jandem]]

Attachment: Bug 1725348 part 1 - Remove proto argument from some array allocation functions. r?jonco!

All callers just want Array.prototype as proto. This lets us optimize more in
later patches.

Comment 2

[Jan de Mooij [:jandem]]

Attachment: Bug 1725348 part 2 - Remove proto argument from NewDensePartlyAllocatedArray. r?jonco!

Depends on D122421

Comment 3

[Jan de Mooij [:jandem]]

Attachment: Bug 1725348 part 3 - Simplify NewDenseCopiedArray by relying on assertions in initDenseElements. r?jonco!

Depends on D122422

Comment 4

[Jan de Mooij [:jandem]]

Attachment: Bug 1725348 part 4 - Remove proto argument from NewDenseCopiedArray. r?jonco!

Depends on D122423

Comment 5

[Jan de Mooij [:jandem]]

Attachment: Bug 1725348 part 5 - Remove unnecessary code in ArrayConstructorImpl. r?jonco!

A nullptr proto is already interpreted as "use the builtin Array.prototype" by
the constructing path, so we can rely on this also for the non-constructing case.

Depends on D122424

Comment 6

[Jan de Mooij [:jandem]]

Attachment: Bug 1725348 part 6 - Rewrite NewArray to be based on the per-global cached shape. r?jonco!

This is more robust and simpler than the NewObjectCache.

Change AddLengthProperty to add the property directly to the shape, instead of
defining the property on the object and then getting its shape.

Depends on D122425

Comment 7

[Jan de Mooij [:jandem]]

Attachment: Bug 1725348 part 7 - Simplify CreateArrayPrototype by using NewArrayWithProto. r?jonco!

This requires moving some code around because of the function definition order.

Depends on D122426

Comment 8

[Jan de Mooij [:jandem]]

Attachment: Bug 1725348 part 8 - Rename arrayShape to arrayShapeWithDefaultProto. r?jonco!

This is more descriptive and more consistent with code added in the previous patch.

Depends on D122427

Comment 9

[Jan de Mooij [:jandem]]

Attachment: Bug 1725348 part 9 - Inline finishCreateArray in its sole caller. r?jonco!

Depends on D122428

Comment 10

[Jan de Mooij [:jandem]]

Attachment: Bug 1725348 part 10 - Inline createArrayInternal in its sole caller. r?jonco!

Depends on D122429

Comment 11

[Jan de Mooij [:jandem]]

Attachment: Bug 1725348 part 11 - Make ArrayObject::createArray more consistent with other allocation functions. r?jonco!

Rename to just "create" and add missing call to debugCheckNewObject for consistency
with JSFunction, NativeObject, TypedObject.

Depends on D122430

Comment 12

[Jan de Mooij [:jandem]]

Attachment: Bug 1725348 part 12 - Optimize Array allocation with slotSpan == 0 a bit more. r?jonco!

Depends on D122431

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a206f62950d1
part 1 - Remove proto argument from some array allocation functions. r=jonco
https://hg.mozilla.org/integration/autoland/rev/9e97aae394d9
part 2 - Remove proto argument from NewDensePartlyAllocatedArray. r=jonco
https://hg.mozilla.org/integration/autoland/rev/36f23bd84226
part 3 - Simplify NewDenseCopiedArray by relying on assertions in initDenseElements. r=jonco
https://hg.mozilla.org/integration/autoland/rev/b4d45fca754d
part 4 - Remove proto argument from NewDenseCopiedArray. r=jonco
https://hg.mozilla.org/integration/autoland/rev/9c26c077837b
part 5 - Remove unnecessary code in ArrayConstructorImpl. r=jonco
https://hg.mozilla.org/integration/autoland/rev/5efae97ecff4
part 6 - Rewrite NewArray to be based on the per-global cached shape. r=jonco
https://hg.mozilla.org/integration/autoland/rev/1a867a5dadda
part 7 - Simplify CreateArrayPrototype by using NewArrayWithProto. r=jonco
https://hg.mozilla.org/integration/autoland/rev/f1938f3d0a49
part 8 - Rename arrayShape to arrayShapeWithDefaultProto. r=jonco
https://hg.mozilla.org/integration/autoland/rev/c1f94239d8b1
part 9 - Inline finishCreateArray in its sole caller. r=jonco
https://hg.mozilla.org/integration/autoland/rev/e74a2be001f2
part 10 - Inline createArrayInternal in its sole caller. r=jonco
https://hg.mozilla.org/integration/autoland/rev/a0ec11ff5ceb
part 11 - Make ArrayObject::createArray more consistent with other allocation functions. r=jonco
https://hg.mozilla.org/integration/autoland/rev/793d0261e6bf
part 12 - Optimize Array allocation with slotSpan == 0 a bit more. r=jonco

Comment 14

[Atila Butkovits]

https://hg.mozilla.org/mozilla-central/rev/a206f62950d1
https://hg.mozilla.org/mozilla-central/rev/9e97aae394d9
https://hg.mozilla.org/mozilla-central/rev/36f23bd84226
https://hg.mozilla.org/mozilla-central/rev/b4d45fca754d
https://hg.mozilla.org/mozilla-central/rev/9c26c077837b
https://hg.mozilla.org/mozilla-central/rev/5efae97ecff4
https://hg.mozilla.org/mozilla-central/rev/1a867a5dadda
https://hg.mozilla.org/mozilla-central/rev/f1938f3d0a49
https://hg.mozilla.org/mozilla-central/rev/c1f94239d8b1
https://hg.mozilla.org/mozilla-central/rev/e74a2be001f2
https://hg.mozilla.org/mozilla-central/rev/a0ec11ff5ceb
https://hg.mozilla.org/mozilla-central/rev/793d0261e6bf