Move property information from shapes to property maps
Categories
(Core :: JavaScript Engine, task, P3)
Tracking
()
| Tracking | Status | |
|---|---|---|
| firefox91 | --- | fixed |
People
(Reporter: jandem, Assigned: jandem)
References
Details
Attachments
(21 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 | |
|
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 | |
|
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 |
See bug 1704430.
Filing this as separate bug because there's more work to do after this part lands.
|
Assignee | |
Comment 1•3 years ago
|
||
|
|
Assignee | |
Comment 2•3 years ago
|
||
Depends on D117301
|
|
Assignee | |
Comment 3•3 years ago
|
||
Depends on D117302
|
|
Assignee | |
Comment 4•3 years ago
|
||
This will for example be used in the PropMapTable which maps from PropertyKey to
map + index.
Depends on D117303
|
|
Assignee | |
Comment 5•3 years ago
|
||
The overload taking a shape will be removed in a later patch.
Depends on D117304
|
|
Assignee | |
Comment 6•3 years ago
|
||
Mpas will store an array of PropertyInfos, add a private constructor to make
that work.
Also add a CompactPropertyInfo type that occupies two bytes instead of four. This
will be used for compact maps.
Depends on D117305
|
|
Assignee | |
Comment 7•3 years ago
|
||
Depends on D117306
|
|
Assignee | |
Comment 8•3 years ago
|
||
This adds most of the layout code and basic operations. Subsequent patches will
implement methods for mutating maps.
Depends on D117307
|
|
Assignee | |
Comment 9•3 years ago
|
||
Depends on D117308
|
|
Assignee | |
Comment 10•3 years ago
|
||
Depends on D117309
|
|
Assignee | |
Comment 11•3 years ago
|
||
This corresponds to the fast path we currently have in SetIntegrityLevel, but
makes it more of a core operation.
The old Shape-based implementation doesn't support dictionary objects, but this
patch adds code for dictionary maps because it's easy to implement and is much
faster than the generic seal/freeze code.
Depends on D117310
|
|
Assignee | |
Comment 12•3 years ago
|
||
Depends on D117311
|
|
Assignee | |
Comment 13•3 years ago
|
||
This will replace what we currently have in ShouldConvertToDictionary.
The HadElementsAccess object flag we currently use there will be removed
in a later patch.
Depends on D117312
|
|
Assignee | |
Comment 14•3 years ago
|
||
This is mostly removing code that's now implemented in property maps:
45 files changed, 1469 insertions(+), 2932 deletions(-)
See also the new SMDOC comment in Shape.h
In CacheIR.cpp remove the previous shape check. This is no longer necessary
after this code got rewritten a while ago.
Depends on D117313
|
|
Assignee | |
Comment 15•3 years ago
|
||
The jit-test was creating objects with dense elements. Change this to use a
non-numeric property name.
The devtools test was failing because shapes no longer show up in the visible part
of the memory allocation table (it's more down the list because there are fewer
shapes). Use PropMap instead.
Depends on D117314
|
|
Assignee | |
Comment 16•3 years ago
|
||
Also rename the IN_DICTIONARY flag to IS_DICTIONARY.
Shapes are no longer "part of" a dictionary list.
Depends on D117315
|
|
Assignee | |
Comment 17•3 years ago
|
||
Depends on D117316
|
|
Assignee | |
Comment 18•3 years ago
|
||
Depends on D117317
|
|
Assignee | |
Comment 19•3 years ago
|
||
Depends on D117318
|
|
Assignee | |
Comment 20•3 years ago
|
||
Also add a SWEEP_PROP_MAP phase because SWEEP_SHAPE still has other uses.
Depends on D117319
|
|
Assignee | |
Comment 21•3 years ago
|
||
Shapes no longer form a tree.
Depends on D117320
|
|
Assignee | |
Updated•3 years ago
|
|
||
Comment 22•3 years ago
|
||
Pushed by jdemooij@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/be3cce91c2ea part 1 - Add TenuredCellWithFlags. r=jonco https://hg.mozilla.org/integration/autoland/rev/fd06a92d7dca part 2 - Add PropMap GC thing. r=jonco https://hg.mozilla.org/integration/autoland/rev/384413c51058 part 3 - Add SMDOC comment for property maps. r=jonco https://hg.mozilla.org/integration/autoland/rev/afc818c6239f part 4 - Add (Shared)PropMapAndIndex tagged pointer type. r=jonco https://hg.mozilla.org/integration/autoland/rev/3f72d711cb8e part 5 - Add GetObjectFlagsForNewProperty overload that takes JSClass and ObjectFlags instead of shape. r=jonco https://hg.mozilla.org/integration/autoland/rev/a891a6588180 part 6 - Prepare PropertyInfo for property maps. r=jonco https://hg.mozilla.org/integration/autoland/rev/1b079a0a9674 part 7 - Add PropertyKey::isVoid. r=jonco https://hg.mozilla.org/integration/autoland/rev/4e9ac24171fc part 8 - Add more property map code. r=jonco https://hg.mozilla.org/integration/autoland/rev/3cd630145cbe part 9 - Implement operations for shared maps. r=jonco https://hg.mozilla.org/integration/autoland/rev/262d59c1f079 part 10 - Implement operations for dictionary maps. r=jonco https://hg.mozilla.org/integration/autoland/rev/42d02f0c0c49 part 11 - Add fast path for sealing/freezing properties. r=jonco https://hg.mozilla.org/integration/autoland/rev/b78abd24ecc6 part 12 - Add memory reporting for property maps. r=jonco https://hg.mozilla.org/integration/autoland/rev/01be02fe5a41 part 13 - Add heuristics for converting a shared map with many properties to a dictionary map. r=jonco
|
||
Comment 23•3 years ago
|
||
| bugherder | ||
https://hg.mozilla.org/mozilla-central/rev/be3cce91c2ea
https://hg.mozilla.org/mozilla-central/rev/fd06a92d7dca
https://hg.mozilla.org/mozilla-central/rev/384413c51058
https://hg.mozilla.org/mozilla-central/rev/afc818c6239f
https://hg.mozilla.org/mozilla-central/rev/3f72d711cb8e
https://hg.mozilla.org/mozilla-central/rev/a891a6588180
https://hg.mozilla.org/mozilla-central/rev/1b079a0a9674
https://hg.mozilla.org/mozilla-central/rev/4e9ac24171fc
https://hg.mozilla.org/mozilla-central/rev/3cd630145cbe
https://hg.mozilla.org/mozilla-central/rev/262d59c1f079
https://hg.mozilla.org/mozilla-central/rev/42d02f0c0c49
https://hg.mozilla.org/mozilla-central/rev/b78abd24ecc6
https://hg.mozilla.org/mozilla-central/rev/01be02fe5a41
|
|
Assignee | |
Updated•3 years ago
|
|
|
||
Comment 24•3 years ago
|
||
Pushed by jdemooij@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/701a2c0f6706 part 14 - Change shapes to be based on property maps. r=jonco https://hg.mozilla.org/integration/autoland/rev/5b1e0b26ed64 part 15 - Fix tests. r=jonco https://hg.mozilla.org/integration/autoland/rev/5a58688c3bdf part 16 - Rename inDictionary to isDictionary. r=jonco https://hg.mozilla.org/integration/autoland/rev/3cf4b32dd4c9 part 17 - Add PropMap::dump and fix Shape::dump. r=jonco https://hg.mozilla.org/integration/autoland/rev/06d244d45354 part 18 - Implement NativeObject::checkShapeConsistency. r=jonco https://hg.mozilla.org/integration/autoland/rev/372a12f1d72f part 19 - Remove now-unused JSID_EMPTY. r=jonco https://hg.mozilla.org/integration/autoland/rev/d54b4b9f3e64 part 20 - Rename Shape to PropMap in a few places. r=jonco https://hg.mozilla.org/integration/autoland/rev/b6290ce477dc part 21 - Rename memory reporter for tree shapes to shared shapes. r=jonco
|
||
Comment 25•3 years ago
|
||
| bugherder | ||
https://hg.mozilla.org/mozilla-central/rev/701a2c0f6706
https://hg.mozilla.org/mozilla-central/rev/5b1e0b26ed64
https://hg.mozilla.org/mozilla-central/rev/5a58688c3bdf
https://hg.mozilla.org/mozilla-central/rev/3cf4b32dd4c9
https://hg.mozilla.org/mozilla-central/rev/06d244d45354
https://hg.mozilla.org/mozilla-central/rev/372a12f1d72f
https://hg.mozilla.org/mozilla-central/rev/d54b4b9f3e64
https://hg.mozilla.org/mozilla-central/rev/b6290ce477dc
|
||
Comment 26•3 years ago
|
||
== Change summary for alert #30437 (as of Tue, 22 Jun 2021 10:36:33 GMT) ==
Improvements:
| Ratio | Suite | Test | Platform | Options | Absolute values (old vs new) |
|---|---|---|---|---|---|
| 6% | JS | macosx1015-64-shippable-qr | 90,328,506.81 -> 85,305,639.95 | ||
| 5% | JS | linux1804-64-shippable-qr | 88,365,054.60 -> 83,825,931.98 | ||
| 5% | JS | macosx1015-64-shippable | 91,238,111.79 -> 86,617,203.39 | ||
| 5% | JS | windows10-64-shippable | 89,238,429.68 -> 85,148,835.48 | ||
| 5% | JS | windows10-64-shippable-qr | 89,314,457.27 -> 85,261,739.40 | ||
| ... | ... | ... | ... | ... | ... |
| 4% | Base Content JS | linux1804-64-shippable-qr | 2,253,376.67 -> 2,173,972.67 |
For up to date results, see: https://treeherder.mozilla.org/perfherder/alerts?id=30437
|
|
Assignee | |
Comment 27•3 years ago
|
||
== Change summary for alert #30503 (as of Thu, 01 Jul 2021 11:59:02 GMT) ==
Improvements:
| Ratio | Suite | Test | Platform | Options | Absolute values (old vs new) |
|---|---|---|---|---|---|
| 2% | outlook | loadtime | macosx1015-64-shippable-qr | warm webrender | 291.98 -> 285.54 |
| 1% | outlook | loadtime | macosx1015-64-shippable-qr | warm webrender | 291.65 -> 287.33 |
For up to date results, see: https://treeherder.mozilla.org/perfherder/alerts?id=30503
Description
•