Refactor XDRScript to better align with current JSScript struct
Categories
(Core :: JavaScript Engine, enhancement, P3)
Tracking
()
| Tracking | Status | |
|---|---|---|
| firefox67 | --- | fixed |
People
(Reporter: tcampbell, Assigned: tcampbell)
References
Details
Attachments
(9 files)
|
47 bytes,
text/x-phabricator-request
|
Details | Review | |
|
47 bytes,
text/x-phabricator-request
|
Details | Review | |
|
47 bytes,
text/x-phabricator-request
|
Details | Review | |
|
47 bytes,
text/x-phabricator-request
|
Details | Review | |
|
47 bytes,
text/x-phabricator-request
|
Details | Review | |
|
47 bytes,
text/x-phabricator-request
|
nbp
:
feedback+
|
Details | Review |
|
47 bytes,
text/x-phabricator-request
|
Details | Review | |
|
47 bytes,
text/x-phabricator-request
|
Details | Review | |
|
47 bytes,
text/x-phabricator-request
|
Details | Review |
js::XDRScript has envolved over the years into a complex mess as JSScript has changed. With Bug 1485347 cleaning up a lot of stuff in the data structure, it makes sense to apply a similar structure to XDRScript (and later CopyScript).
The motivation is to make XDR more robust and make it easy to change JSScript for content-overhead and generated-interpreter projects.
|
Assignee | |
Comment 1•5 years ago
|
||
|
|
Assignee | |
Comment 2•5 years ago
|
||
Depends on D19005
|
|
Assignee | |
Comment 3•5 years ago
|
||
Also change JSTryNote::kind to uint32_t to absorb alignment padding of
structure.
Depends on D19006
|
|
Assignee | |
Comment 4•5 years ago
|
||
Depends on D19007
|
|
Assignee | |
Comment 5•5 years ago
|
||
Depends on D19008
|
|
Assignee | |
Comment 6•5 years ago
|
||
Depends on D19009
|
|
Assignee | |
Comment 7•5 years ago
|
||
Comment on attachment 9042185 [details]
Bug 1525924 - Part 6: Factor out PrivateScriptData::XDR
Nicolas, I'm rewriting XDRScript in this stack to better match our data structures. Any major objections or strong desire to review? Otherwise I've reviewed a lot of Jan's recent JSScript changes so I'll make him review these tedious cleanups.
|
|
Assignee | |
Comment 8•5 years ago
|
||
Comment on attachment 9042185 [details]
Bug 1525924 - Part 6: Factor out PrivateScriptData::XDR
(It works better when I include people's names... )
See Comment 7.
|
|
Assignee | |
Comment 9•5 years ago
|
||
Make the method static so that in a follow-up the allocation of the
object can be absorbed in the method.
|
|
Assignee | |
Comment 10•5 years ago
|
||
This moves the allocation of the ScriptSource object into
ScriptSource::XDR instead of being done in the caller. A
MutableHandle<ScriptSourceHolder> is used to manage both
ScriptSource::refCount as well as tracing Atoms from BinAST.
Depends on D19163
|
|
Assignee | |
Comment 11•5 years ago
|
||
This makes XDRScript better follow the field layout of JSScript.
|
|
Assignee | |
Comment 12•5 years ago
|
||
This series of changes cleans up XDRScript to a lot more organized and much better aligned with the current data structures. The next set of changes (in another bug) will probably change XDRScript/CopyScript/JSScript/etc at the same time to clarify initialization sequences to be less error-prone.
|
||
Comment 13•5 years ago
|
||
Pushed by tcampbell@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/34b6c82e60ce Part 1: Factor out SharedScriptData::XDR r=jandem https://hg.mozilla.org/integration/autoland/rev/582ab25b0a61 Part 2: Factor out WithScope::XDR r=jandem https://hg.mozilla.org/integration/autoland/rev/496594078fd3 Part 3: Factor out JSTryNote/ScopeNote::XDR r=jandem https://hg.mozilla.org/integration/autoland/rev/8045fef98288 Part 4: Factor out js::XDRScope r=jandem https://hg.mozilla.org/integration/autoland/rev/3dde34aa913c Part 5: Factor out js::XDRInnerObject r=jandem https://hg.mozilla.org/integration/autoland/rev/99c5523b079b Part 6: Factor out PrivateScriptData::XDR r=jandem https://hg.mozilla.org/integration/autoland/rev/294323a1a8e6 Part 7: Make ScriptSource::performXDR a static method r=jandem https://hg.mozilla.org/integration/autoland/rev/634ba2074e16 Part 8: Move allocation into ScriptSource::XDR r=jandem https://hg.mozilla.org/integration/autoland/rev/df6a7d40a383 Part 9: Reorder steps of XDRScript r=jandem
|
||
Comment 14•5 years ago
|
||
Comment on attachment 9042185 [details]
Bug 1525924 - Part 6: Factor out PrivateScriptData::XDR
Nice refactoring! This would also make things easier to understand.
|
||
Comment 15•5 years ago
|
||
| bugherder | ||
https://hg.mozilla.org/mozilla-central/rev/34b6c82e60ce
https://hg.mozilla.org/mozilla-central/rev/582ab25b0a61
https://hg.mozilla.org/mozilla-central/rev/496594078fd3
https://hg.mozilla.org/mozilla-central/rev/8045fef98288
https://hg.mozilla.org/mozilla-central/rev/3dde34aa913c
https://hg.mozilla.org/mozilla-central/rev/99c5523b079b
https://hg.mozilla.org/mozilla-central/rev/294323a1a8e6
https://hg.mozilla.org/mozilla-central/rev/634ba2074e16
https://hg.mozilla.org/mozilla-central/rev/df6a7d40a383
Description
•