Remove JSScript cloning mechanism in favour of sharing Stencils
Categories
(Core :: JavaScript Engine, task, P3)
Tracking
()
| Tracking | Status | |
|---|---|---|
| firefox94 | --- | fixed |
People
(Reporter: tcampbell, Assigned: arai)
References
(Blocks 1 open bug)
Details
Attachments
(3 files)
We currently use the CloneScript API for TI-heuristics and for sharing script loader artifacts between Realms. With WarpBuilder removing TI, and Stencil being a better format for script loaders to use, we can remove the entire CloneScript mechanism and simplify a lot of duplicated code.
|
Reporter | |
Updated•4 years ago
|
|
|
Reporter | |
Updated•4 years ago
|
|
Assignee | |
Updated•3 years ago
|
|
|
Assignee | |
Comment 1•3 years ago
|
||
There are 2 entry points for cloning functions:
CloneObject: removed in bug 1688794 patchCloneGlobalScript: used byCloneAndExecuteScriptandExecuteInExtensibleLexicalEnvironment
With the former removed, only CloneObject and CloneFunctionAndScript can be removed, but CloneScriptIntoFunction and remaining functions are also used by CloneGlobalScript.
So, to remove remaining functions, we need to rewrite gecko code that requires cloning JSScript into different realm.
here's the detailed call graph around cloning
CloneObject # removed in bug 1688794
|
+-> CloneFunctionAndScript
|
+-> CloneScriptIntoFunction <-------------------------------------+
| |
+-> FunctionScope::clone |
| | |
| +-> Scope::maybeCloneEnvironmentShape <--------------+ |
| | |
+-> Scope::clone | |
| | | |
| +-> Scope::maybeCloneEnvironmentShape -------------->+ |
| ^ |
+-> CopyScriptImpl <--------------------------------------------+
| | | |
+-> PrivateScriptData::Clone | | |
| | | |
+-> CloneScriptObject | | |
| | | | |
| +-> CloneScriptRegExpObject | | |
| | | | |
| +-> CloneInnerInterpretedFunction | | |
| | | | | |
| | +-> CloneScriptIntoFunction ------------+ |
| | | |
| +-> DeepCloneObjectLiteral <-----------+ | |
| | | | |
| +-> DeepCloneValue | | |
| | | | |
| +-> DeepCloneObjectLiteral --+ | |
| | |
+-> Scope::clone | |
| | |
+-> Scope::maybeCloneEnvironmentShape ----+ |
|
|
CloneGlobalScript |
| ^ |
| | |
| +- CloneAndExecuteScript (without env) |
| | ^ |
| | | |
| | +- mozilla::dom::PrototypeDocumentContentSink::ExecuteScript |
| | | |
| | +- ShellCloneAndExecuteScript (testing function) |
| | | |
| | +- mozilla::dom::PrecompiledScript::ExecuteInGlobal |
| | | |
| | +- mozJSComponentLoader::ObjectForLocation |
| | | |
| | +- EvalScript |
| | |
| +- CloneAndExecuteScript (with env) |
| | ^ |
| | | |
| | +- nsMessageManagerScriptExecutor::LoadScriptInternal |
| | | |
| | +- EvalScript |
| | |
| +- ExecuteInExtensibleLexicalEnvironment |
| ^ |
| | |
| +- ExecuteInFrameScriptEnvironment |
| | ^ |
| | | |
| | +- nsMessageManagerScriptExecutor::LoadScriptInternal |
| | | |
| | +- EvalReturningScope (testing function) |
| | |
| +- JS::ExecuteInJSMEnvironment (with env) |
| ^ |
| | |
| +- JS::ExecuteInJSMEnvironment (without env) |
| | ^ |
| | | |
| | +- mozJSComponentLoader::ObjectForLocation |
| | | |
| | +- EvalScript |
| | |
| +- EvalScript |
| |
+-> ScriptSourceObject::clone |
| |
+-> GlobalScope::clone |
| |
+-> CopyScriptImpl -------------------------------------------------------+
|
|
Assignee | |
Comment 2•3 years ago
|
||
and the source of JSScript passed to CloneAndExecuteScript and ExecuteInExtensibleLexicalEnvironment are StartupCache and ScriptPreloader.
So, this is blocked by legacy XDR removal, and also rewriting the caching mechanism around both of them with Stencil.
|
|
Reporter | |
Comment 3•3 years ago
|
||
Thanks for mapping this out. I believe many (but not all) of these paths don't actually occur in practice (eg we are already in the correct Realm). As a starting point, it might make sense to fix mozilla::dom::PrecompiledScript::ExecuteInGlobal behaviour since none of the existing prototypes/patches have a proposed solution for it.
|
|
Reporter | |
Comment 4•3 years ago
|
||
It looks like ExecuteInExtensibleLexicalEnvironment callers are all same realm (after a single jsshell case is fixed).
https://treeherder.mozilla.org/jobs?repo=try&group_state=expanded&revision=2d3a4d0b424bcff3d219c1930f0a8c5290b77729
|
|
Assignee | |
Comment 5•3 years ago
|
||
Depends on D121258
|
|
Assignee | |
Comment 6•3 years ago
|
||
Depends on D121259
|
|
Assignee | |
Comment 7•3 years ago
|
||
Depends on D121260
|
|
Assignee | |
Comment 8•3 years ago
|
||
(In reply to Ted Campbell [:tcampbell] from comment #4)
It looks like
ExecuteInExtensibleLexicalEnvironmentcallers are all same realm (after a single jsshell case is fixed).
https://treeherder.mozilla.org/jobs?repo=try&group_state=expanded&revision=2d3a4d0b424bcff3d219c1930f0a8c5290b77729
Thank you!
I'll reflect it into bug 1718536 patch
|
||
Updated•3 years ago
|
|
|
||
Updated•3 years ago
|
|
|
||
Updated•3 years ago
|
Pushed by arai_a@mac.com: https://hg.mozilla.org/integration/autoland/rev/6890d49eb664 Part 1: Remove CloneGlobalScript. r=tcampbell https://hg.mozilla.org/integration/autoland/rev/b600e75facb7 Part 2: Remove script cloning. r=tcampbell https://hg.mozilla.org/integration/autoland/rev/76d1ebb7c773 Part 3: Simplify ScriptSourceObject. r=tcampbell,jandem
|
||
Comment 10•3 years ago
|
||
| bugherder | ||
https://hg.mozilla.org/mozilla-central/rev/6890d49eb664
https://hg.mozilla.org/mozilla-central/rev/b600e75facb7
https://hg.mozilla.org/mozilla-central/rev/76d1ebb7c773
Description
•