1599634 - Handle AsmJS Parsing without allocating on the GC

Status: RESOLVED FIXED

(Core :: JavaScript Engine, task, P1)

Description

[Matthew Gaudet (he/him) [:mgaudet]]

There are certain aspects of the AsmJS that have been special-cased for the purposes of making forward progress on this project, but in order to finish we will have to handle AsmJS eventually

Comment 1

[Matthew Gaudet (he/him) [:mgaudet]]

Note to whomever handles this: One important point is NewAsmJSModuleFunction

Comment 2

[Matthew Gaudet (he/him) [:mgaudet]]

The case is nowhere near as hopeless or concerning as I had previously thought. Further investigation suggests the key aspects are here: https://searchfox.org/mozilla-central/rev/e7fc3314b0dfcc61fb7dc7a09f9555660a2eafc5/js/src/wasm/AsmJS.cpp#7094,7101,7111,7121

1. SharedModule is not a GC type, but instead a RefPtr to a non-GC’d type, which already has its ownership handed over to the WasmModuleObject
2. The WasmModuleObject creation only needs the shared module and context. This is great news
3. The allocation of the module function -does- currently take the FunctionBox, but from this needs literally only the name, lambda status, and the nargs. From that it calls NewNativeConstructor. Cool! We can handle this nicely.
4. After the clobber we do need to pass a couple of IsAsmJSModule calls, but that should be not too hard to work around.

Comment 3

[Ted Campbell [:tcampbell]]

The analysis in Comment 2 seems to hold and I have a prototype working now. Patches soon!

Comment 4

[Ted Campbell [:tcampbell]]

Attachment: Bug 1599634 - Stop pre-allocating JSFunction for standalone compiles

Use InitialFunctionFlags to compute flags consistently. That requires
plumbing FunctionSyntaxKind in for the standalone-function case. Preserve the
existing expression-vs-statement decisions. We move the InitialFunctionFlags
definition earlier in file but it is unchanged.

Stop allocating a function before parsing and rely on the frontend to do that
for us. This gives more consistent behaviour with asm.js and inner functions.

We need to explicitly call initEnvironment function now since the Stencil
instantiation code does not do it for us, while NewScriptedFunction did.

Comment 5

[Ted Campbell [:tcampbell]]

Attachment: Bug 1599634 - Move the JS::WasmModule definition to own header

Also add JS::WasmModule::createObjectForAsmJS which is similar to
createObject but does not set the WasmModule prototype. Also mark these
methods as const so they work with the SharedModule definition.

Depends on D78087

Comment 6

[Ted Campbell [:tcampbell]]

Attachment: Bug 1599634 - Defer JSFunction allocation for asm.js modules

Avoid allocating the JSFunction / WasmModuleObject for asm.js during parsing,
but continue to generate the JS::WasmModule (which does not use GC). Add a
map to the CompilationInfo to keep track of these modules until stencils are
being instantiated.

Depends on D78088

Comment 7

[Ted Campbell [:tcampbell]]

TODO: Remove the dummyFunction code from asm.js frontend. FunctionBox no longer requires it.

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/afada3b4e234
Stop pre-allocating JSFunction for standalone compiles r=mgaudet
https://hg.mozilla.org/integration/autoland/rev/834b400f034f
Move the JS::WasmModule definition to own header r=bbouvier

Comment 9

[Dorel Luca [:dluca]]

https://hg.mozilla.org/mozilla-central/rev/afada3b4e234
https://hg.mozilla.org/mozilla-central/rev/834b400f034f

Comment 10

[Ted Campbell [:tcampbell]]

Attachment: Bug 1599634 - Remove dummyFunction from asm.js. r?bbouvier!,djvj!

These days, FunctionBox can be created without a JSFunction which is also
suitable for the asm.js dummyFunction case. Remove the function and pass name
and flags directly to the FunctionBox.

We set flags to INTERPRETED_NORMAL since that is only type of function
allowed at this point in the asm.js context.

Depends on D78089

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ff4b77aae2f9
Defer JSFunction allocation for asm.js modules r=djvj
https://hg.mozilla.org/integration/autoland/rev/57fdda3872e8
Remove dummyFunction from asm.js. r=bbouvier,djvj

Comment 12

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/ff4b77aae2f9
https://hg.mozilla.org/mozilla-central/rev/57fdda3872e8