Closed Bug 1773319 Opened 3 years ago Closed 2 years ago

Add public Stencil APIs to compile without JSContext

Categories

(Core :: JavaScript Engine, task, P3)

Product:
Core
Component:
JavaScript Engine
Type:
task

Tracking

()

Status:
RESOLVED FIXED
Milestone:
113 Branch
Tracking Flags:
Tracking Status
firefox113 --- fixed

People

(Reporter: arai, Assigned: bthrall)

References

(Blocks 2 open bugs)

Details

(Whiteboard: [orb:m2])

Attachments

(20 files, 2 obsolete files)

Bug 1773319 - Make cx parameter to CompileGlobalScriptToStencilAndMaybeInstantiate optional r=arai!
48 bytes, text/x-phabricator-request
Details | Review
Bug 1773319 - Intro API to create/destroy FrontendContext, ParseScript r=arai!
48 bytes, text/x-phabricator-request
Details | Review
Bug 1773319 - Rename maybeCx parameter of frontend::PrepareForInstantiate r=arai!
48 bytes, text/x-phabricator-request
Details | Review
Bug 1773319 - Add CompilationInput parameter to ParseGlobalScript r=arai!
48 bytes, text/x-phabricator-request
Details | Review
Bug 1773319 - Handle Global enclosingScope in CompilationInput::dump() r=arai!
48 bytes, text/x-phabricator-request
Details | Review
Bug 1773319 - Add PrepareForInstantiate() to API r=arai!
48 bytes, text/x-phabricator-request
Details | Review
Bug 1773319 - Use ParseGlobalScript in compileToStencil testing function, add specific tests r=arai!
48 bytes, text/x-phabricator-request
Details | Review
Bug 1773319 - Check failure propagation out of ParseGlobalScript() r=arai!
48 bytes, text/x-phabricator-request
Details | Review
Bug 1773319 - Fix segfault when populating delazification cache r=arai!
48 bytes, text/x-phabricator-request
Details | Review
Bug 1773319 - Rename CompileGlobalScriptToStencil r=arai!
48 bytes, text/x-phabricator-request
Details | Review
Bug 1773319 - Pass LifoAlloc to ParseModuleToStencil() as parameter r=arai!
48 bytes, text/x-phabricator-request
Details | Review
Bug 1773319 - Rename optional maybeCx parameters r=arai!
48 bytes, text/x-phabricator-request
Details | Review
Bug 1773319 - Get supportedImportAssertions from FrontendContext r=arai!
48 bytes, text/x-phabricator-request
Details | Review
Bug 1773319 - Add CompileModuleScriptToStencil() and use it r=arai!
48 bytes, text/x-phabricator-request
Details | Review
Bug 1773319 - Compile nonsyntactic scopes with FrontendContext only r=arai!
48 bytes, text/x-phabricator-request
Details | Review
Bug 1773319 - Add JS::SetSupportedImportAssertions(FrontendContext*) to API r=arai!
48 bytes, text/x-phabricator-request
Details | Review
Bug 1773319 - Rework FrontendContex::gcSafeCallback so rooting hazard analysis understands it r=arai!
48 bytes, text/x-phabricator-request
Details | Review
Bug 1773319 - Use wrapper around CompilationInput instead of UniquePtr r=arai!
48 bytes, text/x-phabricator-request
Details | Review
Bug 1773319 - Assert if ScriptSource::setDisplayURL() is called more than once r=arai!
48 bytes, text/x-phabricator-request
Details | Review
Bug 1773319 - Only define isGCSafe for DEBUG builds
48 bytes, text/x-phabricator-request
Details | Review

The idea is to move the thread handling from JS helper thread to embedding, and let the embedding trigger compilation itself off-thread, instead of calling the off-thread API from the main thread and waiting for the callback.

In order to move the off-thread parsing from JS helper thread to embedding, we need to expose internal functions called in helper threads, as public APIs.

This depends on removing JSContext-dependency from the parsing.

The target is internal functions that's called by ParseTasks

and maybe some small helpers for initialization etc.

Severity: -- → N/A
Priority: -- → P3
Depends on: 1788709
Blocks: 1532644
Depends on: 1799960

Need to fill out jsapi-test for more details and use cases.

AsyncParseGlobalScript() has a stackLimit parameter because I don't expect to
know what the limit for the TaskController task is, or even that the function
is being run by TaskController.

Bug 1759123 and bug 1799960 are cleaning up code that is no longer used, this API doesn't require that they are done.

Assignee: nobody → bthrall
Status: NEW → ASSIGNED
No longer depends on: 1759123, 1799960
Attachment #9313594 - Attachment description: WIP: Bug 1773319 - Intro API to create/destroy FrontendContext, AsyncScriptParse → WIP: Bug 1773319 - Intro API to create/destroy FrontendContext, ParseScript

The CompilationInput will be needed for PrepareForInstantiate().

Depends on D168680

The only failure cases for PrepareForInstantiate() are OOM, which will be
checked in a separate commit.

Depends on D168682

Since CompileGlobalScriptToStencil(FrontendContext*...) is a thin wrapper
around CompileGlobalScriptToStencil(JSContext*...), it makes sense for the
names to match.

compileToStencil should match as well, since the script is not just parsed,
but actually compiled to a stencil.

jsapi-test testCompileNonsyntactic's test name conflicted with
testCompileScript's, so I renamed it to more precisely match what it is
testing.

Depends on D169133

Attachment #9316436 - Attachment description: WIP: Bug 1773319 - Add shell function parseToStencil and test → WIP: Bug 1773319 - Use ParseGlobalScript in compileToStencil testing function, add specific tests
Attachment #9316439 - Attachment description: WIP: Bug 1773319 - Rename CompileGlobalScriptToStencil, compileToStencil → WIP: Bug 1773319 - Rename CompileGlobalScriptToStencil

Heading toward making the JSContext parameter optional.

Depends on D169134

By default, FrontendContext will not list any supportedImportAssertions, but
there is setSupportedImportAssumptions() to set that list once (just like
JS::SetSupportedImportAssumptions() for JSRuntime).

Depends on D169656

I didn't see a way to detect nonsyntactic scope in the Stencil, but there was
an easy way on the instantiated script.

Depends on D169658

No longer blocks: 1532644
See Also: → 1816776
Attachment #9313593 - Attachment description: WIP: Bug 1773319 - Make cx parameter to CompileGlobalScriptToStencilAndMaybeInstantiate optional → Bug 1773319 - Make cx parameter to CompileGlobalScriptToStencilAndMaybeInstantiate optional r=arai!
Attachment #9313594 - Attachment description: WIP: Bug 1773319 - Intro API to create/destroy FrontendContext, ParseScript → Bug 1773319 - Intro API to create/destroy FrontendContext, ParseScript r=arai!
Attachment #9315615 - Attachment description: WIP: Bug 1773319 - Rename maybeCx parameter of frontend::PrepareForInstantiate → Bug 1773319 - Rename maybeCx parameter of frontend::PrepareForInstantiate r=arai!
Attachment #9315617 - Attachment description: WIP: Bug 1773319 - Add CompilationInput parameter to ParseGlobalScript → Bug 1773319 - Add CompilationInput parameter to ParseGlobalScript r=arai!
Attachment #9315618 - Attachment description: WIP: Bug 1773319 - Handle Global enclosingScope in CompilationInput::dump() → Bug 1773319 - Handle Global enclosingScope in CompilationInput::dump() r=arai!
Attachment #9315619 - Attachment description: WIP: Bug 1773319 - Add PrepareForInstantiate() to API → Bug 1773319 - Add PrepareForInstantiate() to API r=arai!
Attachment #9316436 - Attachment description: WIP: Bug 1773319 - Use ParseGlobalScript in compileToStencil testing function, add specific tests → Bug 1773319 - Use ParseGlobalScript in compileToStencil testing function, add specific tests r=arai!
Attachment #9316437 - Attachment description: WIP: Bug 1773319 - Check failure propagation out of ParseGlobalScript() → Bug 1773319 - Check failure propagation out of ParseGlobalScript() r=arai!
Attachment #9316438 - Attachment description: WIP: Bug 1773319 - Fix segfault when populating delazification cache → Bug 1773319 - Fix segfault when populating delazification cache r=arai!
Attachment #9316439 - Attachment description: WIP: Bug 1773319 - Rename CompileGlobalScriptToStencil → Bug 1773319 - Rename CompileGlobalScriptToStencil r=arai!
Attachment #9317384 - Attachment description: WIP: Bug 1773319 - Pass LifoAlloc to ParseModuleToStencil() as parameter → Bug 1773319 - Pass LifoAlloc to ParseModuleToStencil() as parameter r=arai!
Attachment #9317385 - Attachment description: WIP: Bug 1773319 - Rename optional maybeCx parameters → Bug 1773319 - Rename optional maybeCx parameters r=arai!
Attachment #9317386 - Attachment description: WIP: Bug 1773319 - Get supportedImportAssertions from FrontendContext → Bug 1773319 - Get supportedImportAssertions from FrontendContext r=arai!
Attachment #9317387 - Attachment description: WIP: Bug 1773319 Add CompileModuleScriptToStencil() and use it → Bug 1773319 Add CompileModuleScriptToStencil() and use it r=arai!
Attachment #9317496 - Attachment description: WIP: Bug 1773319 - Compile nonsyntactic scopes with FrontendContext only → Bug 1773319 - Compile nonsyntactic scopes with FrontendContext only r=arai!
Attachment #9317387 - Attachment description: Bug 1773319 Add CompileModuleScriptToStencil() and use it r=arai! → Bug 1773319 - Add CompileModuleScriptToStencil() and use it r=arai!
Attachment #9318201 - Attachment description: Bug 1773319 - Add JS_SetSupportedImportAssertions to API r=arai! → Bug 1773319 - Add JS::SetSupportedImportAssertions(FrontendContext*) to API r=arai!
Pushed by bthrall@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/ce294fa9953c Make cx parameter to CompileGlobalScriptToStencilAndMaybeInstantiate optional r=arai https://hg.mozilla.org/integration/autoland/rev/c10a596d7395 Intro API to create/destroy FrontendContext, ParseScript r=arai https://hg.mozilla.org/integration/autoland/rev/1068cb71a7bf Rename maybeCx parameter of frontend::PrepareForInstantiate r=arai https://hg.mozilla.org/integration/autoland/rev/6c952cee28e2 Add CompilationInput parameter to ParseGlobalScript r=arai https://hg.mozilla.org/integration/autoland/rev/8bdbbd322d34 Handle Global enclosingScope in CompilationInput::dump() r=arai https://hg.mozilla.org/integration/autoland/rev/83d4b82795fe Add PrepareForInstantiate() to API r=arai https://hg.mozilla.org/integration/autoland/rev/4e7e1ec10ed2 Use ParseGlobalScript in compileToStencil testing function, add specific tests r=arai https://hg.mozilla.org/integration/autoland/rev/26a8fc40437f Check failure propagation out of ParseGlobalScript() r=arai https://hg.mozilla.org/integration/autoland/rev/c3fd0107003b Fix segfault when populating delazification cache r=arai https://hg.mozilla.org/integration/autoland/rev/3005fd755990 Rename CompileGlobalScriptToStencil r=arai https://hg.mozilla.org/integration/autoland/rev/7f773c7ee0fd Pass LifoAlloc to ParseModuleToStencil() as parameter r=arai https://hg.mozilla.org/integration/autoland/rev/50c3ae074be3 Rename optional maybeCx parameters r=arai https://hg.mozilla.org/integration/autoland/rev/c4e12157c001 Get supportedImportAssertions from FrontendContext r=arai https://hg.mozilla.org/integration/autoland/rev/d1f138fae2c4 Add CompileModuleScriptToStencil() and use it r=arai https://hg.mozilla.org/integration/autoland/rev/55c0f82e8645 Compile nonsyntactic scopes with FrontendContext only r=arai https://hg.mozilla.org/integration/autoland/rev/432d50b65ee6 Add JS::SetSupportedImportAssertions(FrontendContext*) to API r=arai

Backed out for causing hazard bustages in TestingFunctions.cpp

  • Backout link
  • Push with failures
  • Failure Log
  • Failure line: TEST-UNEXPECTED-FAIL | hazards | unrooted 'stencilInput' of type 'mozilla::UniquePtr<js::frontend::CompilationInput, JS::DeletePolicy<js::frontend::CompilationInput> >' live across GC call at js/src/builtin/TestingFunctions.cpp:6373
Flags: needinfo?(bthrall)
Flags: needinfo?(bthrall)
Depends on: 1819470
No longer depends on: 1819470

Previously, rooting hazard analysis could report calls to gcSafeCallback as
triggering a GC because of the two calls to callback(). Now it understands that
the single call to callback() will not trigger a GC.

Depends on D170083

The rooting hazard analysis assumes function pointer arguments might trigger
GC, but none of the functions used for mustMatchToken() will do so.

Depends on D172333

The rooting hazard analysis assumes function pointer arguments might trigger
GC, but none of the functions used for matchInteger() or
matchIntegerAfterFirstDigit() will do so.

Depends on D172334

Embedders would not be able to use UniquePtr<CompilationInput> since
CompilationInput is opaque to them.

This also resolves a rooting hazard with the UniquePtr because the JS script
might have a 'uses asm' directive, which the hazard analysis reports could be a
problem if CompilationInput has GC pointers. We know the CompilationInput has
no GC pointers, however because we are only compiling to Stencil.

Further, CompilationStorage::isValid() only returns true if its
CompilationInput contains no GC pointers.

Depends on D172335

There is no code path currently which will call setDisplayURL() if it has
already been set. The previous code to report a warning could not execute, and
caused the rooting hazard analysis to report a hazard when setDisplayURL() was
called from the CompileScript API. Removing the call to WarnNumberLatin1()
avoids the hazard.

Depends on D172336

Attachment #9322524 - Attachment is obsolete: true
Attachment #9322523 - Attachment is obsolete: true
Whiteboard: [orb:m2]
Blocks: 1826106
Pushed by bthrall@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/e87b56a40133 Make cx parameter to CompileGlobalScriptToStencilAndMaybeInstantiate optional r=arai https://hg.mozilla.org/integration/autoland/rev/4d9649652cad Intro API to create/destroy FrontendContext, ParseScript r=arai https://hg.mozilla.org/integration/autoland/rev/ac1327af1c8f Rename maybeCx parameter of frontend::PrepareForInstantiate r=arai https://hg.mozilla.org/integration/autoland/rev/feb45ca89d78 Add CompilationInput parameter to ParseGlobalScript r=arai https://hg.mozilla.org/integration/autoland/rev/804f4087add7 Handle Global enclosingScope in CompilationInput::dump() r=arai https://hg.mozilla.org/integration/autoland/rev/1ab9e2b34325 Add PrepareForInstantiate() to API r=arai https://hg.mozilla.org/integration/autoland/rev/063afc991fd0 Use ParseGlobalScript in compileToStencil testing function, add specific tests r=arai https://hg.mozilla.org/integration/autoland/rev/6c3d1aae58f7 Check failure propagation out of ParseGlobalScript() r=arai https://hg.mozilla.org/integration/autoland/rev/3aa2a44717d2 Fix segfault when populating delazification cache r=arai https://hg.mozilla.org/integration/autoland/rev/2ce2fb7d0ad2 Rename CompileGlobalScriptToStencil r=arai https://hg.mozilla.org/integration/autoland/rev/5399b1528245 Pass LifoAlloc to ParseModuleToStencil() as parameter r=arai https://hg.mozilla.org/integration/autoland/rev/3be347da0c96 Rename optional maybeCx parameters r=arai https://hg.mozilla.org/integration/autoland/rev/2606ae2fe37c Get supportedImportAssertions from FrontendContext r=arai https://hg.mozilla.org/integration/autoland/rev/e7d0f790f097 Add CompileModuleScriptToStencil() and use it r=arai https://hg.mozilla.org/integration/autoland/rev/dbe2b23d6e54 Compile nonsyntactic scopes with FrontendContext only r=arai https://hg.mozilla.org/integration/autoland/rev/fa8d4e0d012b Add JS::SetSupportedImportAssertions(FrontendContext*) to API r=arai https://hg.mozilla.org/integration/autoland/rev/6b4bb6a9527e Rework FrontendContex::gcSafeCallback so rooting hazard analysis understands it r=arai https://hg.mozilla.org/integration/autoland/rev/4ef61ac2f4fa Use wrapper around CompilationInput instead of UniquePtr r=arai https://hg.mozilla.org/integration/autoland/rev/c62f9fb3170f Assert if ScriptSource::setDisplayURL() is called more than once r=arai

It is only used in MOZ_ASSERT, so it was unused in non-DEBUG builds.

Attachment #9326948 - Attachment description: WIP: Bug 1773319 - Only define isGCSafe for DEBUG builds → Bug 1773319 - Only define isGCSafe for DEBUG builds
Pushed by nbeleuzu@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/97ea5a6d852f Only define isGCSafe for DEBUG builds
Regressions: 1826553
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: