1528964 - js::CopyScript spring cleaning

Status: RESOLVED FIXED

(Core :: JavaScript Engine, enhancement, P1)

Description

[Ted Campbell [:tcampbell]]

Similar to Bug 1525505, the CopyScript function needs a lot of cleanup to better match the rest of the JSScript initialization code. This is currently making it hard to do major changes around JSScript safely and efficiently.

Comment 1

[Ted Campbell [:tcampbell]]

Attachment: Bug 1528964 - Part 1: Absorb CreateEmptyScriptForClone into CopyScript

Comment 2

[Ted Campbell [:tcampbell]]

Attachment: Bug 1528964 - Part 2: Split part of CopyScript into PrivateScriptData::Clone

This is a code-motion patch. Note that the order of initialization of
PrivateScriptData vs JSScript fields changes but the reordered
operations are all infallible/gc-free so this is not observable.

Depends on D20332

Comment 3

[Ted Campbell [:tcampbell]]

Attachment: Bug 1528964 - Part 3: Use createPrivateScriptData in PrivateScriptData::Clone

Also use PrivateScriptData directly instead of through the JSScript.

Depends on D20333

Comment 4

[Ted Campbell [:tcampbell]]

Attachment: Bug 1528964 - Part 4: Add SharedScriptData::markForCrossZone

Depends on D20334

Comment 5

[Ted Campbell [:tcampbell]]

Attachment: Bug 1528964 - Part 5: Organize CopyScript similar to XDRScript

The JSScript fields are now initialized before the script data objects
to better align with XDRScript.

Depends on D20335

Comment 6

[Ted Campbell [:tcampbell]]

Attachment: Bug 1528964 - Part 6: Preserve column while cloning scripts

This is an older bug that was hidden in the old CopyScript
implementation.

Depends on D20336

Comment 7

[Ted Campbell [:tcampbell]]

Attachment: Bug 1528964 - Part 7: Don't preserve needsArgsObj flag in CopyScript

Similar to Bug 1526324, we should avoid one-off optimizations that share
runtime data between script copies.

Depends on D20337

Comment 8

[Ted Campbell [:tcampbell]]

Follow-up work for other bugs:

• Revisit the scope cloning logic to simplify it more
• Minimize the three-way diff between BytecodeCompiler/XDRScript/CopyScript now that they are reasonably close

Comment 9

[Ted Campbell [:tcampbell]]

Attachment: Bug 1528964 - Part 8: CopyScript should directly return result script

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a0192ce3b15d
Part 1: Absorb CreateEmptyScriptForClone into CopyScript r=jandem
https://hg.mozilla.org/integration/autoland/rev/cfcd89883d1e
Part 2: Split part of CopyScript into PrivateScriptData::Clone r=jandem
https://hg.mozilla.org/integration/autoland/rev/05d3db762616
Part 3: Use createPrivateScriptData in PrivateScriptData::Clone r=jandem
https://hg.mozilla.org/integration/autoland/rev/de9bb567ad44
Part 4: Add SharedScriptData::markForCrossZone r=jandem
https://hg.mozilla.org/integration/autoland/rev/b89f5efa1bf9
Part 5: Organize CopyScript similar to XDRScript r=jandem
https://hg.mozilla.org/integration/autoland/rev/f0e44d5ad172
Part 6: Preserve column while cloning scripts r=jandem
https://hg.mozilla.org/integration/autoland/rev/348464a06c14
Part 7: Don't preserve needsArgsObj flag in CopyScript r=jandem
https://hg.mozilla.org/integration/autoland/rev/73749ff4f2ef
Part 8: CopyScript should directly return result script r=jandem

Comment 11

[Daniel Varga [:dvarga]]

https://hg.mozilla.org/mozilla-central/rev/a0192ce3b15d
https://hg.mozilla.org/mozilla-central/rev/cfcd89883d1e
https://hg.mozilla.org/mozilla-central/rev/05d3db762616
https://hg.mozilla.org/mozilla-central/rev/de9bb567ad44
https://hg.mozilla.org/mozilla-central/rev/b89f5efa1bf9
https://hg.mozilla.org/mozilla-central/rev/f0e44d5ad172
https://hg.mozilla.org/mozilla-central/rev/348464a06c14
https://hg.mozilla.org/mozilla-central/rev/73749ff4f2ef