1662152 - Remove JSScript cloning mechanism in favour of sharing Stencils

Status: RESOLVED FIXED

(Core :: JavaScript Engine, task, P3)

Description

[Ted Campbell [:tcampbell]]

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.

Comment 1

[Tooru Fujisawa [:arai]]

There are 2 entry points for cloning functions:

• CloneObject : removed in bug 1688794 patch
• CloneGlobalScript : used by CloneAndExecuteScript and ExecuteInExtensibleLexicalEnvironment

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 -------------------------------------------------------+

Comment 2

[Tooru Fujisawa [:arai]]

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.

Comment 3

[Ted Campbell [:tcampbell]]

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.

Comment 4

[Ted Campbell [:tcampbell]]

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

Comment 5

[Tooru Fujisawa [:arai]]

Attachment: Bug 1662152 - Part 1: Remove CloneGlobalScript. r?tcampbell!

Depends on D121258

Comment 6

[Tooru Fujisawa [:arai]]

Attachment: Bug 1662152 - Part 2: Remove script cloning. r?tcampbell!

Depends on D121259

Comment 7

[Tooru Fujisawa [:arai]]

Attachment: Bug 1662152 - Part 3: Simplify ScriptSourceObject. r?tcampbell!

Depends on D121260

Comment 8

[Tooru Fujisawa [:arai]]

(In reply to Ted Campbell [:tcampbell] from comment #4)

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

Thank you!
I'll reflect it into bug 1718536 patch

Comment 9

[Pulsebot]

Pushed by [e-mail]:
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

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/6890d49eb664
https://hg.mozilla.org/mozilla-central/rev/b600e75facb7
https://hg.mozilla.org/mozilla-central/rev/76d1ebb7c773