1559275 - Minor cleanups in CompileOptions handling

Status: RESOLVED FIXED

(Core :: JavaScript Engine, enhancement, P2)

Description

[Ted Campbell [:tcampbell]]

Track discardSources on the CompileOptions (but initialize it from the realm behaviour). This simplifies things when trying to pre-compile exotic scripts like the self-hosting script.

Comment 1

[Ted Campbell [:tcampbell]]

Attachment: Bug 1559275 - Add TransistiveCompileOptions::discardSource. r?jandem

Check realm flags at when creating CompileOptions rather than during
compilation. This is helpful for pre-compiling self-hosting code.

Comment 2

[Ted Campbell [:tcampbell]]

Attachment: Bug 1559275 - Remove unused CompileOptions::isProbablySystemCode. r?jandem

Depends on D34976

Comment 3

[Ted Campbell [:tcampbell]]

While here, I also removed isProbablySystemCode since it is unused in compiler.

I tried to also cleanup the parser's check for Realm::disableLazyParser but it didn't seem like a good idea to store it in the lazyParse option just yet.

One issue that comes up is that the AsyncScriptLoader component as affected by Bug 1409782. We don't currently use async loading or discard sources outside of tests. This should probably be cleaned up in follow-ups.

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/248e294630c1
Add CompileOptions::discardSource. r=jandem
https://hg.mozilla.org/integration/autoland/rev/41762079fd4c
Remove unused CompileOptions::isProbablySystemCode. r=jandem

Comment 5

[Ted Campbell [:tcampbell]]

Attachment: Bug 1559275 - Remove unused CompileOptions::allowSyntaxParser. r?jandem

Comment 6

[Ted Campbell [:tcampbell]]

Attachment: Bug 1559275 - Use JS::CompileOptions in AsyncScriptCompiler. r?kmag

The OwningCompileOptions constructor doesn't perform completion default
initialization so use normal CompileOptions to set up options and then
clone into the OwningCompileOptions. This AsyncScriptLoader module
doesn't seem to have any uses right now anyways.

Comment 7

[Ted Campbell [:tcampbell]]

Attachment: Bug 1559275 - Only initialize JS::OwningCompileOptions by copying. r?jandem

All uses of OwningCompileOptions now are initialized from copy() so
remove the now unused setters.

Comment 9

[Dorel Luca [:dluca]]

Backed out 2 changesets (bug 1559275) for Mochitest failures in js/xpconnect/tests/chrome/test_discardSystemSource.xul. CLOSED TREE

Log:
https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=251944066&repo=autoland&lineNumber=1591

Push with failures:
https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=41762079fd4cdbee9f3b14f895d9e18061d82eec

Backout:
https://hg.mozilla.org/integration/autoland/rev/696560d760bb4dd5138e49dfa5b798278bed1c8f

Comment 10

[Ted Campbell [:tcampbell]]

Looks like the order I land these patches is more important than I realized. I'll do further testing, but it looks like the 'fix' is make sure the OwningCompileOptions changes (two patches) should come first.

Comment 11

[Ted Campbell [:tcampbell]]

Attachment: Bug 1559275 - Add TransistiveCompileOptions::forceFullParse. r?jandem

Check VM and Realm options that may disable syntax parsing up front in
the CompileOptions constructor. This is needed to make the frontend
closer to a pure-function.

Depends on D34976

Comment 12

[Ted Campbell [:tcampbell]]

Attachment: Bug 1559275 - Avoid OwningCompileOptions accessors in js-rust glue. r?jandem

Depends on D35071

Comment 13

[Ted Campbell [:tcampbell]]

Sorry for all the churn. I think this is all passing tests for me. After final reviews, I'll land it.

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/3c83d9936c2c
Use JS::CompileOptions in AsyncScriptCompiler. r=kmag
https://hg.mozilla.org/integration/autoland/rev/cd6b4126a1b6
Avoid OwningCompileOptions accessors in js-rust glue. r=jandem
https://hg.mozilla.org/integration/autoland/rev/96acb8c9df07
Only initialize JS::OwningCompileOptions by copying. r=jandem
https://hg.mozilla.org/integration/autoland/rev/ef106f22e7e5
Remove unused CompileOptions::isProbablySystemCode. r=jandem
https://hg.mozilla.org/integration/autoland/rev/901f51251ea9
Remove unused CompileOptions::allowSyntaxParser. r=jandem
https://hg.mozilla.org/integration/autoland/rev/28a43f9cc6b1
Add TransistiveCompileOptions::discardSource. r=jandem
https://hg.mozilla.org/integration/autoland/rev/960c9f0926e9
Add TransistiveCompileOptions::forceFullParse. r=jandem

Comment 15

[Raul Gurzau (:RaulG)]

https://hg.mozilla.org/mozilla-central/rev/3c83d9936c2c
https://hg.mozilla.org/mozilla-central/rev/cd6b4126a1b6
https://hg.mozilla.org/mozilla-central/rev/96acb8c9df07
https://hg.mozilla.org/mozilla-central/rev/ef106f22e7e5
https://hg.mozilla.org/mozilla-central/rev/901f51251ea9
https://hg.mozilla.org/mozilla-central/rev/28a43f9cc6b1
https://hg.mozilla.org/mozilla-central/rev/960c9f0926e9