Bug 166480

Summary: WebAssembly: test compile / instantiate in a Worker
Product: WebKit Reporter: JF Bastien <jfbastien>
Component: JavaScriptCoreAssignee: Nobody <webkit-unassigned>
Status: RESOLVED CONFIGURATION CHANGED    
Severity: Normal CC: jfbastien, keith_miller, sbarati, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 166481, 168264    
Bug Blocks: 161709    

Description JF Bastien 2016-12-26 09:35:43 PST
I've heard from someone trying out WebAssembly that they current use compile / instantiate in a worker because some implementations aren't asynchronous. They want to do work on the main thread while compiling, so a Worker makes sense. They then transfer the Module / Instance back to the main thread.

We should have a test that this works well.

Related:
 - WebAssembly: Make WebAssembly.compile truly asynchronous https://bugs.webkit.org/show_bug.cgi?id=166016
 - WebAssembly: Implement the WebAssembly.instantiate API https://bugs.webkit.org/show_bug.cgi?id=165982
Comment 1 Saam Barati 2016-12-26 11:34:36 PST
(In reply to comment #0)
> I've heard from someone trying out WebAssembly that they current use compile
> / instantiate in a worker because some implementations aren't asynchronous.
> They want to do work on the main thread while compiling, so a Worker makes
> sense. They then transfer the Module / Instance back to the main thread.
> 
> We should have a test that this works well.
> 
> Related:
>  - WebAssembly: Make WebAssembly.compile truly asynchronous
> https://bugs.webkit.org/show_bug.cgi?id=166016
>  - WebAssembly: Implement the WebAssembly.instantiate API
> https://bugs.webkit.org/show_bug.cgi?id=165982

I think this should work as expected regardless of the compile API being asynchronous internally.
Comment 2 Saam Barati 2016-12-26 11:35:09 PST
(In reply to comment #1)
> (In reply to comment #0)
> > I've heard from someone trying out WebAssembly that they current use compile
> > / instantiate in a worker because some implementations aren't asynchronous.
> > They want to do work on the main thread while compiling, so a Worker makes
> > sense. They then transfer the Module / Instance back to the main thread.
> > 
> > We should have a test that this works well.
> > 
> > Related:
> >  - WebAssembly: Make WebAssembly.compile truly asynchronous
> > https://bugs.webkit.org/show_bug.cgi?id=166016
> >  - WebAssembly: Implement the WebAssembly.instantiate API
> > https://bugs.webkit.org/show_bug.cgi?id=165982
> 
> I think this should work as expected regardless of the compile API being
> asynchronous internally.

Also, I agree we should have a test for this.
Comment 3 JF Bastien 2016-12-26 11:40:55 PST
(In reply to comment #2)
> (In reply to comment #1)
> > (In reply to comment #0)
> > > I've heard from someone trying out WebAssembly that they current use compile
> > > / instantiate in a worker because some implementations aren't asynchronous.
> > > They want to do work on the main thread while compiling, so a Worker makes
> > > sense. They then transfer the Module / Instance back to the main thread.
> > > 
> > > We should have a test that this works well.
> > > 
> > > Related:
> > >  - WebAssembly: Make WebAssembly.compile truly asynchronous
> > > https://bugs.webkit.org/show_bug.cgi?id=166016
> > >  - WebAssembly: Implement the WebAssembly.instantiate API
> > > https://bugs.webkit.org/show_bug.cgi?id=165982
> > 
> > I think this should work as expected regardless of the compile API being
> > asynchronous internally.
> 
> Also, I agree we should have a test for this.

Right, I want to make sure postMessage-ing Module / Instance / Memory / Table all work as expected.
Comment 4 Saam Barati 2016-12-26 12:08:21 PST
(In reply to comment #3)
> (In reply to comment #2)
> > (In reply to comment #1)
> > > (In reply to comment #0)
> > > > I've heard from someone trying out WebAssembly that they current use compile
> > > > / instantiate in a worker because some implementations aren't asynchronous.
> > > > They want to do work on the main thread while compiling, so a Worker makes
> > > > sense. They then transfer the Module / Instance back to the main thread.
> > > > 
> > > > We should have a test that this works well.
> > > > 
> > > > Related:
> > > >  - WebAssembly: Make WebAssembly.compile truly asynchronous
> > > > https://bugs.webkit.org/show_bug.cgi?id=166016
> > > >  - WebAssembly: Implement the WebAssembly.instantiate API
> > > > https://bugs.webkit.org/show_bug.cgi?id=165982
> > > 
> > > I think this should work as expected regardless of the compile API being
> > > asynchronous internally.
> > 
> > Also, I agree we should have a test for this.
> 
> Right, I want to make sure postMessage-ing Module / Instance / Memory /
> Table all work as expected.

Interesting. Is that the expected behavior? This is unlike anything we do elsewhere. We don't postMessage heap cells since each worker is in a different VM
Comment 5 JF Bastien 2016-12-26 12:09:32 PST
(In reply to comment #4)
> (In reply to comment #3)
> > (In reply to comment #2)
> > > (In reply to comment #1)
> > > > (In reply to comment #0)
> > > > > I've heard from someone trying out WebAssembly that they current use compile
> > > > > / instantiate in a worker because some implementations aren't asynchronous.
> > > > > They want to do work on the main thread while compiling, so a Worker makes
> > > > > sense. They then transfer the Module / Instance back to the main thread.
> > > > > 
> > > > > We should have a test that this works well.
> > > > > 
> > > > > Related:
> > > > >  - WebAssembly: Make WebAssembly.compile truly asynchronous
> > > > > https://bugs.webkit.org/show_bug.cgi?id=166016
> > > > >  - WebAssembly: Implement the WebAssembly.instantiate API
> > > > > https://bugs.webkit.org/show_bug.cgi?id=165982
> > > > 
> > > > I think this should work as expected regardless of the compile API being
> > > > asynchronous internally.
> > > 
> > > Also, I agree we should have a test for this.
> > 
> > Right, I want to make sure postMessage-ing Module / Instance / Memory /
> > Table all work as expected.
> 
> Interesting. Is that the expected behavior? This is unlike anything we do
> elsewhere. We don't postMessage heap cells since each worker is in a
> different VM

I don't know what's expected :-D
Comment 6 Saam Barati 2016-12-26 12:21:19 PST
(In reply to comment #5)
> (In reply to comment #4)
> > (In reply to comment #3)
> > > (In reply to comment #2)
> > > > (In reply to comment #1)
> > > > > (In reply to comment #0)
> > > > > > I've heard from someone trying out WebAssembly that they current use compile
> > > > > > / instantiate in a worker because some implementations aren't asynchronous.
> > > > > > They want to do work on the main thread while compiling, so a Worker makes
> > > > > > sense. They then transfer the Module / Instance back to the main thread.
> > > > > > 
> > > > > > We should have a test that this works well.
> > > > > > 
> > > > > > Related:
> > > > > >  - WebAssembly: Make WebAssembly.compile truly asynchronous
> > > > > > https://bugs.webkit.org/show_bug.cgi?id=166016
> > > > > >  - WebAssembly: Implement the WebAssembly.instantiate API
> > > > > > https://bugs.webkit.org/show_bug.cgi?id=165982
> > > > > 
> > > > > I think this should work as expected regardless of the compile API being
> > > > > asynchronous internally.
> > > > 
> > > > Also, I agree we should have a test for this.
> > > 
> > > Right, I want to make sure postMessage-ing Module / Instance / Memory /
> > > Table all work as expected.
> > 
> > Interesting. Is that the expected behavior? This is unlike anything we do
> > elsewhere. We don't postMessage heap cells since each worker is in a
> > different VM
> 
> I don't know what's expected :-D

We won't be able to compile a Module then ship over the module across workers. We will be able to have workers that have their own Modules/Instances/etc that can run independently. We need to think about what happens when we try to ship over an ArrayBuffer for Memory. This should probably not be allowed since it's backed by Wasm::Memory mmaped memory.
Comment 7 JF Bastien 2016-12-26 12:23:51 PST
(In reply to comment #6)
> (In reply to comment #5)
> > (In reply to comment #4)
> > > (In reply to comment #3)
> > > > (In reply to comment #2)
> > > > > (In reply to comment #1)
> > > > > > (In reply to comment #0)
> > > > > > > I've heard from someone trying out WebAssembly that they current use compile
> > > > > > > / instantiate in a worker because some implementations aren't asynchronous.
> > > > > > > They want to do work on the main thread while compiling, so a Worker makes
> > > > > > > sense. They then transfer the Module / Instance back to the main thread.
> > > > > > > 
> > > > > > > We should have a test that this works well.
> > > > > > > 
> > > > > > > Related:
> > > > > > >  - WebAssembly: Make WebAssembly.compile truly asynchronous
> > > > > > > https://bugs.webkit.org/show_bug.cgi?id=166016
> > > > > > >  - WebAssembly: Implement the WebAssembly.instantiate API
> > > > > > > https://bugs.webkit.org/show_bug.cgi?id=165982
> > > > > > 
> > > > > > I think this should work as expected regardless of the compile API being
> > > > > > asynchronous internally.
> > > > > 
> > > > > Also, I agree we should have a test for this.
> > > > 
> > > > Right, I want to make sure postMessage-ing Module / Instance / Memory /
> > > > Table all work as expected.
> > > 
> > > Interesting. Is that the expected behavior? This is unlike anything we do
> > > elsewhere. We don't postMessage heap cells since each worker is in a
> > > different VM
> > 
> > I don't know what's expected :-D
> 
> We won't be able to compile a Module then ship over the module across
> workers. We will be able to have workers that have their own
> Modules/Instances/etc that can run independently. We need to think about
> what happens when we try to ship over an ArrayBuffer for Memory. This should
> probably not be allowed since it's backed by Wasm::Memory mmaped memory.

IIUC they serialize WebAssembly.Module, and transfer it from Worker to main thread. Apparently this works in Chrome / FF. Should we file an issue on this in the design repo?
Comment 8 Saam Barati 2016-12-26 12:27:27 PST
(In reply to comment #7)
> (In reply to comment #6)
> > (In reply to comment #5)
> > > (In reply to comment #4)
> > > > (In reply to comment #3)
> > > > > (In reply to comment #2)
> > > > > > (In reply to comment #1)
> > > > > > > (In reply to comment #0)
> > > > > > > > I've heard from someone trying out WebAssembly that they current use compile
> > > > > > > > / instantiate in a worker because some implementations aren't asynchronous.
> > > > > > > > They want to do work on the main thread while compiling, so a Worker makes
> > > > > > > > sense. They then transfer the Module / Instance back to the main thread.
> > > > > > > > 
> > > > > > > > We should have a test that this works well.
> > > > > > > > 
> > > > > > > > Related:
> > > > > > > >  - WebAssembly: Make WebAssembly.compile truly asynchronous
> > > > > > > > https://bugs.webkit.org/show_bug.cgi?id=166016
> > > > > > > >  - WebAssembly: Implement the WebAssembly.instantiate API
> > > > > > > > https://bugs.webkit.org/show_bug.cgi?id=165982
> > > > > > > 
> > > > > > > I think this should work as expected regardless of the compile API being
> > > > > > > asynchronous internally.
> > > > > > 
> > > > > > Also, I agree we should have a test for this.
> > > > > 
> > > > > Right, I want to make sure postMessage-ing Module / Instance / Memory /
> > > > > Table all work as expected.
> > > > 
> > > > Interesting. Is that the expected behavior? This is unlike anything we do
> > > > elsewhere. We don't postMessage heap cells since each worker is in a
> > > > different VM
> > > 
> > > I don't know what's expected :-D
> > 
> > We won't be able to compile a Module then ship over the module across
> > workers. We will be able to have workers that have their own
> > Modules/Instances/etc that can run independently. We need to think about
> > what happens when we try to ship over an ArrayBuffer for Memory. This should
> > probably not be allowed since it's backed by Wasm::Memory mmaped memory.
> 
> IIUC they serialize WebAssembly.Module, and transfer it from Worker to main
> thread. Apparently this works in Chrome / FF. Should we file an issue on
> this in the design repo?
We should probably support the same serialization. I'm interested to know what happens to the underlying Memory/etc in the worker that created them after they're transferred. Also, what happens to the underlying memory? Is there a spec for this serialization? A better API than using workers would be we should just make the asynchronous APIs truly asynchronous. I don't think that should be too difficult
Comment 9 JF Bastien 2016-12-26 13:02:03 PST
(In reply to comment #8)
> (In reply to comment #7)
> > (In reply to comment #6)
> > > (In reply to comment #5)
> > > > (In reply to comment #4)
> > > > > (In reply to comment #3)
> > > > > > (In reply to comment #2)
> > > > > > > (In reply to comment #1)
> > > > > > > > (In reply to comment #0)
> > > > > > > > > I've heard from someone trying out WebAssembly that they current use compile
> > > > > > > > > / instantiate in a worker because some implementations aren't asynchronous.
> > > > > > > > > They want to do work on the main thread while compiling, so a Worker makes
> > > > > > > > > sense. They then transfer the Module / Instance back to the main thread.
> > > > > > > > > 
> > > > > > > > > We should have a test that this works well.
> > > > > > > > > 
> > > > > > > > > Related:
> > > > > > > > >  - WebAssembly: Make WebAssembly.compile truly asynchronous
> > > > > > > > > https://bugs.webkit.org/show_bug.cgi?id=166016
> > > > > > > > >  - WebAssembly: Implement the WebAssembly.instantiate API
> > > > > > > > > https://bugs.webkit.org/show_bug.cgi?id=165982
> > > > > > > > 
> > > > > > > > I think this should work as expected regardless of the compile API being
> > > > > > > > asynchronous internally.
> > > > > > > 
> > > > > > > Also, I agree we should have a test for this.
> > > > > > 
> > > > > > Right, I want to make sure postMessage-ing Module / Instance / Memory /
> > > > > > Table all work as expected.
> > > > > 
> > > > > Interesting. Is that the expected behavior? This is unlike anything we do
> > > > > elsewhere. We don't postMessage heap cells since each worker is in a
> > > > > different VM
> > > > 
> > > > I don't know what's expected :-D
> > > 
> > > We won't be able to compile a Module then ship over the module across
> > > workers. We will be able to have workers that have their own
> > > Modules/Instances/etc that can run independently. We need to think about
> > > what happens when we try to ship over an ArrayBuffer for Memory. This should
> > > probably not be allowed since it's backed by Wasm::Memory mmaped memory.
> > 
> > IIUC they serialize WebAssembly.Module, and transfer it from Worker to main
> > thread. Apparently this works in Chrome / FF. Should we file an issue on
> > this in the design repo?
> We should probably support the same serialization. I'm interested to know
> what happens to the underlying Memory/etc in the worker that created them
> after they're transferred. Also, what happens to the underlying memory? Is
> there a spec for this serialization? A better API than using workers would
> be we should just make the asynchronous APIs truly asynchronous. I don't
> think that should be too difficult

I believe that only Module is expected to be postMessage-able correctly, and that the details are tied to IndexDB support:
  https://bugs.webkit.org/show_bug.cgi?id=166481
Comment 10 JF Bastien 2016-12-26 13:04:26 PST
At a minimum we'll have to be able to patch vm.topJSWebAssemblyInstance (I'm removing the other wasm things from VM).
Comment 11 Radar WebKit Bug Importer 2017-05-03 09:56:41 PDT
<rdar://problem/31965359>
Comment 12 Saam Barati 2017-05-03 10:17:25 PDT
This is already done.