Store the Shape instead of PlainObject template for NewObject ops
Categories
(Core :: JavaScript Engine, task, P3)
Tracking
()
| Tracking | Status | |
|---|---|---|
| firefox93 | --- | fixed |
People
(Reporter: jandem, Assigned: jandem)
References
Details
Attachments
(8 files)
|
48 bytes,
text/x-phabricator-request
|
Details | Review | |
|
48 bytes,
text/x-phabricator-request
|
Details | Review | |
|
48 bytes,
text/x-phabricator-request
|
Details | Review | |
|
48 bytes,
text/x-phabricator-request
|
Details | Review | |
|
48 bytes,
text/x-phabricator-request
|
Details | Review | |
|
48 bytes,
text/x-phabricator-request
|
Details | Review | |
|
48 bytes,
text/x-phabricator-request
|
Details | Review | |
|
48 bytes,
text/x-phabricator-request
|
Details | Review |
Each NewObject op has a PlainObject template, but we're only using the shape of this object. It's simpler and more efficient to just store a shape in the PrivateScriptData.
I have some patches for this that improve Base Content JS by about 10 KB. They also optimize Stencil instantiation to fill the property map before creating the shape, so we can avoid allocating the intermediate shapes.
|
Assignee | |
Comment 1•3 years ago
|
||
Every caller passes GenericObject.
|
|
Assignee | |
Comment 2•3 years ago
|
||
MNewObject never has a template object if mode == ObjectLiteral. This lets us
remove some code.
Depends on D123090
|
|
Assignee | |
Comment 3•3 years ago
|
||
The XDR and cloning code is the most complicated part unfortunately. This is based on
what we do for plain objects. Hopefully this can all be removed at some point.
Depends on D123091
|
|
Assignee | |
Comment 4•3 years ago
|
||
We still allocate the object in the Stencil instantiation code, and then only
use its shape. This will be fixed in a later patch to allocate only the shape.
Depends on D123092
|
|
Assignee | |
Comment 5•3 years ago
|
||
If these properties will be dense elements, there's no reason to define them on the
template object (or template shape with later patches).
If they're sparse elements, we now don't include them in the object's shape, but
it should be okay to rely on InitElem ops to define them later on.
Depends on D123093
|
|
Assignee | |
Comment 6•3 years ago
|
||
We now no longer allocate the object. We also don't allocate intermediate shapes for
each property because we first fill the map and then allocate the final shape.
Depends on D123095
|
|
Assignee | |
Comment 7•3 years ago
|
||
|
|
Assignee | |
Comment 8•3 years ago
|
||
|
||
Comment 9•3 years ago
|
||
Backed out 8 changesets (Bug 1726533) for causing assertion failures in GlobalObject.h
Backout link: https://hg.mozilla.org/integration/autoland/rev/bad63becefdf64143f3924663ae15fe1250dc6c4
Push with failures, failure log
|
||
Comment 10•3 years ago
|
||
| bugherder | ||
https://hg.mozilla.org/mozilla-central/rev/73d9d271b970
https://hg.mozilla.org/mozilla-central/rev/78f0fc4d86d2
https://hg.mozilla.org/mozilla-central/rev/eb156df63f48
https://hg.mozilla.org/mozilla-central/rev/256cc3db0ae1
https://hg.mozilla.org/mozilla-central/rev/6e629fb82a17
https://hg.mozilla.org/mozilla-central/rev/e6af4c62cf08
https://hg.mozilla.org/mozilla-central/rev/c0c2d32ead72
https://hg.mozilla.org/mozilla-central/rev/1065470b86b3
|
|
Assignee | |
Updated•3 years ago
|
|
||
Comment 11•3 years ago
|
||
Pushed by jdemooij@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/e5b8aa23a72d part 1 - Remove NewObjectKind argument from NewObjectOperation. r=arai https://hg.mozilla.org/integration/autoland/rev/b8e239e5d9f2 part 2 - Remove NewObjectOperationWithTemplate. r=arai https://hg.mozilla.org/integration/autoland/rev/7aab10434b86 part 3 - Add support for storing PlainObject shapes in PrivateScriptData. r=arai https://hg.mozilla.org/integration/autoland/rev/920a837b565d part 4 - Store the shape instead of object for NewObject ops. r=arai https://hg.mozilla.org/integration/autoland/rev/2cb0d1374a19 part 5 - Don't store indexed properties for non-singleton object literals. r=arai https://hg.mozilla.org/integration/autoland/rev/f162e4597ada part 6 - Optimize PlainObject shape allocation for Stencil instantiation. r=arai https://hg.mozilla.org/integration/autoland/rev/3c5d0917c4d9 part 7 - Fix some comments. r=arai
|
||
Comment 12•3 years ago
|
||
== Change summary for alert #31048 (as of Thu, 26 Aug 2021 11:06:57 GMT) ==
Improvements:
| Ratio | Test | Platform | Options | Absolute values (old vs new) |
|---|---|---|---|---|
| 1% | Base Content JS | linux1804-64-shippable-qr | fission | 1,744,168.00 -> 1,734,313.33 |
| 1% | Base Content JS | macosx1015-64-shippable-qr | fission | 1,741,936.00 -> 1,731,994.67 |
| 1% | Base Content JS | windows10-64-2004-shippable-qr | fission | 1,749,123.33 -> 1,739,618.00 |
| 0.30% | Base Content JS | macosx1015-64-shippable-qr | fission | 1,741,936.00 -> 1,736,740.67 |
For up to date results, see: https://treeherder.mozilla.org/perfherder/alerts?id=31048
Description
•