Share more auxiliary data in JSScript
Categories
(Core :: JavaScript Engine, enhancement, P3)
Tracking
()
Tracking | Status | |
---|---|---|
firefox69 | --- | fixed |
People
(Reporter: tcampbell, Assigned: tcampbell)
References
(Blocks 1 open bug)
Details
(Whiteboard: [overhead:20k])
Attachments
(7 files, 1 obsolete file)
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details |
It looks like some fields in JSScript unshared data can be moved to SharedScriptData, such as: JSScript::consts() JSScript::trynotes() JSScript::scopeNotes() JSScript::yieldAndAwaitOffsets() This is expected to be a win for cases where JSScripts are cloned into other compartments, as well as helping Fission efforts.
Assignee | ||
Comment 1•6 years ago
|
||
Additional TODO here: - Rename the data_ / scriptData_ fields to just about anything else - Replace TryNoteArray, etc. return types with mozilla::Span<const T> - The way they handle offsets needs to be reconciled
Comment 2•6 years ago
|
||
Ted, do you have an estimate of how much savings this would have?
Assignee | ||
Comment 3•6 years ago
|
||
Guestimate based on some about:memory data is around 20k. I'm working on this anyways for other code cleanup endeavors of spidermonkey. This also leads into the bytecode-sharing work which has the bulk of savings.
Updated•6 years ago
|
Assignee | ||
Updated•6 years ago
|
Assignee | ||
Comment 4•5 years ago
|
||
Assignee | ||
Comment 6•5 years ago
|
||
Pad the source notes with SRC_NULL such that the code and notes arrays
together maintain uint32_t alignment. This is will later be used to add
optional trailing arrays with uint32_t alignment. It is expected that
the allocators would have not been using these bytes so memory usage
should be neutral.
Depends on D34787
Assignee | ||
Comment 7•5 years ago
|
||
Add a flag into the byte array area. It is inserted before code() so it
will be at a fixed offset once atoms are removed. This will be used to
store flags about optional tail arrays.
Depends on D34788
Assignee | ||
Comment 8•5 years ago
|
||
These arrays don't have any pointers and so should be made shareable.
Depends on D34789
Assignee | ||
Comment 10•5 years ago
|
||
Patches work. This does some excessively clever packing to optimize for fast access of commonly accessed data and avoiding storing sizes for empty arrays (some of which are common). I'm still uncertain if this is a good idea. We should measure how 'optional' some of these optional arrays and make their size/offset always defined if that can reduce complexity.
This conflicts with Bug 1558801 because I forgot I already wrote the same patch (sorry).
Updated•5 years ago
|
Updated•5 years ago
|
Updated•5 years ago
|
Updated•5 years ago
|
Assignee | ||
Comment 11•5 years ago
|
||
Now that PrivateScriptData contains a single array, the PackedSpan
mechanism for packing multiple trailing arrays can be removed.
Depends on D34790
Updated•5 years ago
|
Updated•5 years ago
|
Assignee | ||
Comment 12•5 years ago
|
||
I've cleaned up the code to a point it can be landed. There is currently a 4kB JS regression on awsy-base because of padding issue not recovering removed fields. This will get better after feature changes. The primary motivation for this patch set is to move more data into SharedScriptData for when self-hosting/chrome bytecode can be used directly from binaries/cache.
Assignee | ||
Comment 13•5 years ago
|
||
As PrivateScriptData contains less data, we need to make this test have
more complicated functions so that the test is not sensistive to
allocator rounding. This also removes the binjs variant of test until
they next time they are regenerated.
Comment 14•5 years ago
|
||
Pushed by tcampbell@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/83a5257788cb Make heap-analysis/byteSize-of-scripts jit-test more complex r=jandem,arai https://hg.mozilla.org/integration/autoland/rev/0964e2a0ed29 Add SharedScriptData::offsetToPointer. r=jandem https://hg.mozilla.org/integration/autoland/rev/f6226246197b Pad source notes to target alignment. r=jandem https://hg.mozilla.org/integration/autoland/rev/8718427bcb0f Move resumeOffsets/scopeNotes/tryNotes to SharedScriptData. r=jandem https://hg.mozilla.org/integration/autoland/rev/161f46ffacb7 Remove the PrivateScriptData::PackedSpan mechanism. r=jandem https://hg.mozilla.org/integration/autoland/rev/9624fc4c9e10 Add SharedScriptData::flags. r=jandem https://hg.mozilla.org/integration/autoland/rev/6ded0846247c Avoid storing SharedScriptData metadata for empty arrays. r=jandem
Comment 15•5 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/83a5257788cb
https://hg.mozilla.org/mozilla-central/rev/0964e2a0ed29
https://hg.mozilla.org/mozilla-central/rev/f6226246197b
https://hg.mozilla.org/mozilla-central/rev/8718427bcb0f
https://hg.mozilla.org/mozilla-central/rev/161f46ffacb7
https://hg.mozilla.org/mozilla-central/rev/9624fc4c9e10
https://hg.mozilla.org/mozilla-central/rev/6ded0846247c
Description
•