Closed Bug 1666683 Opened 1 year ago Closed 1 year ago

Check input CompileOptions when decoding, or getting from cache

Categories

(Core :: JavaScript Engine, task, P1)

task

Tracking

()

RESOLVED FIXED
83 Branch
Tracking Status
firefox83 --- fixed

People

(Reporter: arai, Assigned: arai)

References

(Blocks 1 open bug)

Details

Attachments

(12 files, 1 obsolete file)

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
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
Details | Review
47 bytes, text/x-phabricator-request
Details | Review

Depends on D90585

Blocks: stencil-backlog
No longer blocks: stencil-mvp
Attachment #9177295 - Attachment is obsolete: true

In ScriptLoader, same CompileOption is passed to both JS::Compile and JS::DecodeScriptMaybeStencil.
but in other places, callers aren't passing same CompileOption as JS::Compile to decoding API.

  1. JS::DecodeScript doesn't receive CompileOptions.
  2. JS::DecodeMultiOffThreadScripts receives CompileOptions, but the option is shared across multiple scripts that can be compiled with different options

and in remaining consumers, the decoded script is stored into cache and used later, and compilation happens when the cache isn't found.
It means we should check CompileOptions vs ImmutableScriptFlags when getting from the cache, instead of when decoding.

I'll add an API that checks CompileOptions vs ImmutableScriptFlags, for existing consumers.

once we replace the cache to store stencil, we can compare CompileOptions directly between cache and compilation input.

Summary: Check input CompileOptions when decoding → Check input CompileOptions when decoding, or getting from cache

plan:

  1. for JS::DecodeScript, add CompileOptions parameter and modify the consumer to pass the same options as JS::Compile, and check the options vs flags internally
  2. for JS::DecodeMultiOffThreadScripts, modify the consumers to pass options that is consistent as much as possible (but still not all options can be same), and check some options vs flags internally
  3. for JS::DecodeMultiOffThreadScripts consumers, when getting a decoded script from the cache, make it receive CompileOptions and check options vs flags

This adds the parameter, without using it, also without passing the correct
value.
The later patch will change the consumer of those API to pass the correct value,
and then use the passed parameter.

Depends on D92403

Attachment #9179562 - Attachment description: Bug 1666683 - Part 12: Add JS::IsCompileOptionsMatching and check options in ScriptPreloader. r?tcampbell! → Bug 1666683 - Part 12: Add JS::CheckCompileOptionsMatch and check options in ScriptPreloader. r?tcampbell!
Pushed by arai_a@mac.com:
https://hg.mozilla.org/integration/autoland/rev/c5f2032ed919
Part 1: Reorder CompileOptions parameter. r=tcampbell
https://hg.mozilla.org/integration/autoland/rev/a6da1179a589
Part 2: Add CompileOptions parameter to JS::DecodeScript, as a placeholder. r=tcampbell
https://hg.mozilla.org/integration/autoland/rev/777c94a3c977
Part 3: Add options parameter to ScriptPreloader::GetCachedScript. r=tcampbell,kmag
https://hg.mozilla.org/integration/autoland/rev/3014569a17c7
Part 4: Add options parameter to ReadCachedScript in mozJSLoaderUtils. r=tcampbell,kmag
https://hg.mozilla.org/integration/autoland/rev/8ecde54fab31
Part 5: Add options parameter to nsXPConnect::ReadScript. r=tcampbell,kmag
https://hg.mozilla.org/integration/autoland/rev/483e81a2dbd7
Part 6: Add ScriptPreloader::FillCompileOptionsForCachedScript. r=tcampbell,kmag
https://hg.mozilla.org/integration/autoland/rev/3e4b7122f576
Part 7: Fix shell offThreadDecodeScript to use the same option as execute. r=tcampbell
https://hg.mozilla.org/integration/autoland/rev/50cd482cdb64
Part 8: Move options_ field from XDROffThreadDecoder to XDRDecoder. r=tcampbell
https://hg.mozilla.org/integration/autoland/rev/5a6df8bb1ab5
Part 9: Add XDRState::isMultiDecode as a preparation to check options conditionally. r=tcampbell
https://hg.mozilla.org/integration/autoland/rev/240b2e7d6afc
Part 10: Check CompileOptions when decoding XDR. r=tcampbell
https://hg.mozilla.org/integration/autoland/rev/2fbbdd37bb54
Part 11: Add FillImmutableFlagsFromCompileOptionsForTopLevel to centralize CompileOptions-ImmutableScriptFlags interaction. r=tcampbell
https://hg.mozilla.org/integration/autoland/rev/8827caa78683
Part 12: Add JS::CheckCompileOptionsMatch and check options in ScriptPreloader. r=tcampbell,kmag
You need to log in before you can comment on or make changes to this bug.