Bug 170573

Summary: WebAssembly: We should be able to postMessage a JSWebAssemblyModule
Product: WebKit Reporter: Saam Barati <saam>
Component: JavaScriptCoreAssignee: Saam Barati <saam>
Status: RESOLVED FIXED    
Severity: Normal CC: achristensen, alecflett, beidson, benjamin, buildbot, commit-queue, don.olmstead, fpizlo, ggaren, gskachkov, jfbastien, jsbell, keith_miller, mark.lam, msaboff, ossy, rniwa, ticaiolima, ysuzuki
Priority: P2    
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 168264    
Attachments:
Description Flags
WIP
none
WIP
none
WIP
none
WIP
none
patch
none
patch
none
patch
fpizlo: review+, buildbot: commit-queue-
Archive of layout-test-results from ews103 for mac-elcapitan
none
Archive of layout-test-results from ews114 for mac-elcapitan
none
Archive of layout-test-results from ews107 for mac-elcapitan-wk2
none
patch for landing
none
patch for landing
none
patch for landing
buildbot: commit-queue-
Archive of layout-test-results from ews102 for mac-elcapitan
none
Archive of layout-test-results from ews106 for mac-elcapitan-wk2
none
Archive of layout-test-results from ews116 for mac-elcapitan
none
Archive of layout-test-results from ews123 for ios-simulator-wk2
none
patch for landing none

Description Saam Barati 2017-04-06 15:53:43 PDT
...
Comment 1 Saam Barati 2017-04-07 17:48:38 PDT
*** Bug 170632 has been marked as a duplicate of this bug. ***
Comment 2 Saam Barati 2017-04-07 17:49:01 PDT
Created attachment 306557 [details]
WIP
Comment 3 Saam Barati 2017-04-11 08:30:34 PDT
Created attachment 306818 [details]
WIP

I think it works. Need to clean up some code.
Comment 4 Saam Barati 2017-04-11 08:33:52 PDT
I got a WasmBench test to run in a worker and main thread VM concurrently to each other.
Comment 5 JF Bastien 2017-04-11 10:24:05 PDT
(In reply to Saam Barati from comment #4)
> I got a WasmBench test to run in a worker and main thread VM concurrently to
> each other.

🎉
Comment 6 Saam Barati 2017-04-11 12:18:01 PDT
Created attachment 306838 [details]
WIP

Want to see tests run now that worker heap timers should actually GC for certain programs where this wouldn't happen before.
Comment 7 Saam Barati 2017-04-11 15:34:52 PDT
Created attachment 306866 [details]
WIP

Have tests now.
Comment 8 Saam Barati 2017-04-13 11:38:59 PDT
Created attachment 307003 [details]
patch
Comment 9 Saam Barati 2017-04-13 11:43:10 PDT
Created attachment 307004 [details]
patch

rebased
Comment 10 Saam Barati 2017-04-13 11:44:27 PDT
Created attachment 307005 [details]
patch

remove some logging.
Comment 11 Build Bot 2017-04-13 11:46:41 PDT
Attachment 307005 [details] did not pass style-queue:


ERROR: Source/WebCore/ForwardingHeaders/wasm/js/JSWebAssemblyModule.h:0:  No copyright message found.  You should have a line: "Copyright [year] <Copyright Owner>"  [legal/copyright] [5]
ERROR: Source/WebCore/ForwardingHeaders/runtime/PromiseDeferredTimer.h:0:  No copyright message found.  You should have a line: "Copyright [year] <Copyright Owner>"  [legal/copyright] [5]
ERROR: Source/WebCore/ForwardingHeaders/heap/GCActivityCallback.h:0:  No copyright message found.  You should have a line: "Copyright [year] <Copyright Owner>"  [legal/copyright] [5]
ERROR: Source/WebCore/ForwardingHeaders/runtime/JSRunLoopTimer.h:0:  No copyright message found.  You should have a line: "Copyright [year] <Copyright Owner>"  [legal/copyright] [5]
Total errors found: 4 in 42 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 12 Filip Pizlo 2017-04-13 11:48:30 PDT
Comment on attachment 307005 [details]
patch

View in context: https://bugs.webkit.org/attachment.cgi?id=307005&action=review

> Source/JavaScriptCore/ChangeLog:12
> +        This patch adds a callback to JSRunLoopTimer to notify
> +        clients that a timer has been set. This is used inside
> +        WorkerRunLoop in WebCore so that its RunLoop can perform
> +        an iteration when it sees that a timer got set.
> +

It would be super useful to call out in this change log that we are postMessaging modules by just sharing them.
Comment 13 Build Bot 2017-04-13 12:57:48 PDT
Comment on attachment 307005 [details]
patch

Attachment 307005 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/3530057

New failing tests:
workers/wasm-hashset-many.html
workers/wasm-hashset.html
Comment 14 Build Bot 2017-04-13 12:57:50 PDT
Created attachment 307014 [details]
Archive of layout-test-results from ews103 for mac-elcapitan

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews103  Port: mac-elcapitan  Platform: Mac OS X 10.11.6
Comment 15 Build Bot 2017-04-13 13:10:12 PDT
Comment on attachment 307005 [details]
patch

Attachment 307005 [details] did not pass mac-debug-ews (mac):
Output: http://webkit-queues.webkit.org/results/3530062

New failing tests:
workers/wasm-hashset-many.html
workers/wasm-hashset.html
workers/wasm-long-compile.html
workers/wasm-long-compile-many.html
Comment 16 Build Bot 2017-04-13 13:10:13 PDT
Created attachment 307016 [details]
Archive of layout-test-results from ews114 for mac-elcapitan

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews114  Port: mac-elcapitan  Platform: Mac OS X 10.11.6
Comment 17 Build Bot 2017-04-13 13:28:19 PDT
Comment on attachment 307005 [details]
patch

Attachment 307005 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/3530154

New failing tests:
workers/wasm-hashset-many.html
workers/wasm-hashset.html
Comment 18 Build Bot 2017-04-13 13:28:21 PDT
Created attachment 307018 [details]
Archive of layout-test-results from ews107 for mac-elcapitan-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews107  Port: mac-elcapitan-wk2  Platform: Mac OS X 10.11.6
Comment 19 Saam Barati 2017-04-13 13:33:04 PDT
Created attachment 307019 [details]
patch for landing

Marked tests as slow, will see if that appeases EWS.
Also added necessary #ifdefs to make it run on non-wasm builds.
Comment 20 Build Bot 2017-04-13 13:34:44 PDT
Attachment 307019 [details] did not pass style-queue:


ERROR: Source/WebCore/ForwardingHeaders/wasm/js/JSWebAssemblyModule.h:0:  No copyright message found.  You should have a line: "Copyright [year] <Copyright Owner>"  [legal/copyright] [5]
ERROR: Source/WebCore/ForwardingHeaders/runtime/PromiseDeferredTimer.h:0:  No copyright message found.  You should have a line: "Copyright [year] <Copyright Owner>"  [legal/copyright] [5]
ERROR: Source/WebCore/ForwardingHeaders/heap/GCActivityCallback.h:0:  No copyright message found.  You should have a line: "Copyright [year] <Copyright Owner>"  [legal/copyright] [5]
ERROR: Source/WebCore/ForwardingHeaders/runtime/JSRunLoopTimer.h:0:  No copyright message found.  You should have a line: "Copyright [year] <Copyright Owner>"  [legal/copyright] [5]
ERROR: Source/WebCore/bindings/js/SerializedScriptValue.cpp:563:  Wrong number of spaces before statement. (expected: 16)  [whitespace/indent] [4]
ERROR: Source/WebCore/bindings/js/SerializedScriptValue.cpp:1664:  Wrong number of spaces before statement. (expected: 12)  [whitespace/indent] [4]
ERROR: Source/WebCore/bindings/js/SerializedScriptValue.cpp:1689:  Wrong number of spaces before statement. (expected: 12)  [whitespace/indent] [4]
ERROR: Source/WebCore/bindings/js/SerializedScriptValue.cpp:2874:  Wrong number of spaces before statement. (expected: 12)  [whitespace/indent] [4]
Total errors found: 8 in 43 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 21 Saam Barati 2017-04-13 14:08:40 PDT
Created attachment 307022 [details]
patch for landing

try to get non-wasm enabled builds to compile.
Comment 22 Build Bot 2017-04-13 14:10:55 PDT
Attachment 307022 [details] did not pass style-queue:


ERROR: Source/WebCore/ForwardingHeaders/wasm/js/JSWebAssemblyModule.h:0:  No copyright message found.  You should have a line: "Copyright [year] <Copyright Owner>"  [legal/copyright] [5]
ERROR: Source/WebCore/ForwardingHeaders/runtime/PromiseDeferredTimer.h:0:  No copyright message found.  You should have a line: "Copyright [year] <Copyright Owner>"  [legal/copyright] [5]
ERROR: Source/WebCore/ForwardingHeaders/heap/GCActivityCallback.h:0:  No copyright message found.  You should have a line: "Copyright [year] <Copyright Owner>"  [legal/copyright] [5]
ERROR: Source/WebCore/ForwardingHeaders/runtime/JSRunLoopTimer.h:0:  No copyright message found.  You should have a line: "Copyright [year] <Copyright Owner>"  [legal/copyright] [5]
ERROR: Source/WebCore/bindings/js/SerializedScriptValue.cpp:563:  Wrong number of spaces before statement. (expected: 16)  [whitespace/indent] [4]
ERROR: Source/WebCore/bindings/js/SerializedScriptValue.cpp:1664:  Wrong number of spaces before statement. (expected: 12)  [whitespace/indent] [4]
ERROR: Source/WebCore/bindings/js/SerializedScriptValue.cpp:1689:  Wrong number of spaces before statement. (expected: 12)  [whitespace/indent] [4]
ERROR: Source/WebCore/bindings/js/SerializedScriptValue.cpp:2874:  Wrong number of spaces before statement. (expected: 12)  [whitespace/indent] [4]
Total errors found: 8 in 43 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 23 Saam Barati 2017-04-13 14:26:05 PDT
Created attachment 307024 [details]
patch for landing

try to fix the build.
Comment 24 Build Bot 2017-04-13 14:29:20 PDT
Attachment 307024 [details] did not pass style-queue:


ERROR: Source/WebCore/ForwardingHeaders/wasm/js/JSWebAssemblyModule.h:0:  No copyright message found.  You should have a line: "Copyright [year] <Copyright Owner>"  [legal/copyright] [5]
ERROR: Source/WebCore/ForwardingHeaders/runtime/PromiseDeferredTimer.h:0:  No copyright message found.  You should have a line: "Copyright [year] <Copyright Owner>"  [legal/copyright] [5]
ERROR: Source/WebCore/ForwardingHeaders/heap/GCActivityCallback.h:0:  No copyright message found.  You should have a line: "Copyright [year] <Copyright Owner>"  [legal/copyright] [5]
ERROR: Source/WebCore/ForwardingHeaders/runtime/JSRunLoopTimer.h:0:  No copyright message found.  You should have a line: "Copyright [year] <Copyright Owner>"  [legal/copyright] [5]
ERROR: Source/WebCore/bindings/js/SerializedScriptValue.cpp:563:  Wrong number of spaces before statement. (expected: 16)  [whitespace/indent] [4]
ERROR: Source/WebCore/bindings/js/SerializedScriptValue.cpp:1664:  Wrong number of spaces before statement. (expected: 12)  [whitespace/indent] [4]
ERROR: Source/WebCore/bindings/js/SerializedScriptValue.cpp:1689:  Wrong number of spaces before statement. (expected: 12)  [whitespace/indent] [4]
ERROR: Source/WebCore/bindings/js/SerializedScriptValue.cpp:2878:  Wrong number of spaces before statement. (expected: 12)  [whitespace/indent] [4]
Total errors found: 8 in 43 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 25 Build Bot 2017-04-13 15:54:15 PDT
Comment on attachment 307024 [details]
patch for landing

Attachment 307024 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/3530775

New failing tests:
workers/wasm-hashset-many.html
workers/wasm-hashset.html
Comment 26 Build Bot 2017-04-13 15:54:17 PDT
Created attachment 307041 [details]
Archive of layout-test-results from ews102 for mac-elcapitan

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews102  Port: mac-elcapitan  Platform: Mac OS X 10.11.6
Comment 27 Build Bot 2017-04-13 16:07:09 PDT
Comment on attachment 307024 [details]
patch for landing

Attachment 307024 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/3530787

New failing tests:
workers/wasm-hashset-many.html
workers/wasm-hashset.html
Comment 28 Build Bot 2017-04-13 16:07:10 PDT
Created attachment 307043 [details]
Archive of layout-test-results from ews106 for mac-elcapitan-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews106  Port: mac-elcapitan-wk2  Platform: Mac OS X 10.11.6
Comment 29 Build Bot 2017-04-13 16:19:56 PDT
Comment on attachment 307024 [details]
patch for landing

Attachment 307024 [details] did not pass mac-debug-ews (mac):
Output: http://webkit-queues.webkit.org/results/3530789

New failing tests:
workers/wasm-hashset-many.html
workers/wasm-hashset.html
workers/wasm-long-compile-many.html
Comment 30 Build Bot 2017-04-13 16:19:58 PDT
Created attachment 307044 [details]
Archive of layout-test-results from ews116 for mac-elcapitan

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews116  Port: mac-elcapitan  Platform: Mac OS X 10.11.6
Comment 31 Saam Barati 2017-04-13 17:06:20 PDT
I'm adding some slower tests. My tests are timing EWS out.
Comment 32 Build Bot 2017-04-13 17:14:51 PDT
Comment on attachment 307024 [details]
patch for landing

Attachment 307024 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: http://webkit-queues.webkit.org/results/3530905

New failing tests:
workers/wasm-hashset-many.html
workers/wasm-hashset.html
workers/wasm-long-compile.html
workers/wasm-long-compile-many.html
Comment 33 Build Bot 2017-04-13 17:14:56 PDT
Created attachment 307052 [details]
Archive of layout-test-results from ews123 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews123  Port: ios-simulator-wk2  Platform: Mac OS X 10.11.6
Comment 34 Saam Barati 2017-04-13 17:24:40 PDT
Created attachment 307055 [details]
patch for landing

Hopefully tests run fast enough now.
Comment 35 Build Bot 2017-04-13 17:27:40 PDT
Attachment 307055 [details] did not pass style-queue:


ERROR: Source/WebCore/ForwardingHeaders/wasm/js/JSWebAssemblyModule.h:0:  No copyright message found.  You should have a line: "Copyright [year] <Copyright Owner>"  [legal/copyright] [5]
ERROR: Source/WebCore/ForwardingHeaders/runtime/PromiseDeferredTimer.h:0:  No copyright message found.  You should have a line: "Copyright [year] <Copyright Owner>"  [legal/copyright] [5]
ERROR: Source/WebCore/ForwardingHeaders/heap/GCActivityCallback.h:0:  No copyright message found.  You should have a line: "Copyright [year] <Copyright Owner>"  [legal/copyright] [5]
ERROR: Source/WebCore/ForwardingHeaders/runtime/JSRunLoopTimer.h:0:  No copyright message found.  You should have a line: "Copyright [year] <Copyright Owner>"  [legal/copyright] [5]
ERROR: Source/WebCore/bindings/js/SerializedScriptValue.cpp:563:  Wrong number of spaces before statement. (expected: 16)  [whitespace/indent] [4]
ERROR: Source/WebCore/bindings/js/SerializedScriptValue.cpp:1664:  Wrong number of spaces before statement. (expected: 12)  [whitespace/indent] [4]
ERROR: Source/WebCore/bindings/js/SerializedScriptValue.cpp:1689:  Wrong number of spaces before statement. (expected: 12)  [whitespace/indent] [4]
ERROR: Source/WebCore/bindings/js/SerializedScriptValue.cpp:2878:  Wrong number of spaces before statement. (expected: 12)  [whitespace/indent] [4]
Total errors found: 8 in 46 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 36 WebKit Commit Bot 2017-04-13 19:10:24 PDT
Comment on attachment 307055 [details]
patch for landing

Clearing flags on attachment: 307055

Committed r215353: <http://trac.webkit.org/changeset/215353>
Comment 37 WebKit Commit Bot 2017-04-13 19:10:26 PDT
All reviewed patches have been landed.  Closing bug.
Comment 38 Yusuke Suzuki 2017-04-14 05:39:25 PDT
Comment on attachment 307055 [details]
patch for landing

View in context: https://bugs.webkit.org/attachment.cgi?id=307055&action=review

> Source/WebCore/ChangeLog:32
> +        level MessageQueue timeout to be no longer than the next timer fire date.

Just curiious, I have one question. Is there any chance to merge WorkerRunLoop to WTF::RunLoop?
Comment 39 Saam Barati 2017-04-14 08:34:40 PDT
(In reply to Yusuke Suzuki from comment #38)
> Comment on attachment 307055 [details]
> patch for landing
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=307055&action=review
> 
> > Source/WebCore/ChangeLog:32
> > +        level MessageQueue timeout to be no longer than the next timer fire date.
> 
> Just curiious, I have one question. Is there any chance to merge
> WorkerRunLoop to WTF::RunLoop?

I think this would be doable, and perhaps better than what we have now. I need to read WTF::RunLoop to be sure.
Comment 40 Csaba Osztrogonác 2017-04-14 13:01:59 PDT
(In reply to WebKit Commit Bot from comment #36)
> Comment on attachment 307055 [details]
> patch for landing
> 
> Clearing flags on attachment: 307055
> 
> Committed r215353: <http://trac.webkit.org/changeset/215353>

It broke the Apple Mac cmake build:
/Volumes/Data/slave/elcapitan-cmake-debug/build/Source/WebCore/ForwardingHeaders/wasm/js/JSWebAssemblyModule.h:2:10: fatal error: 'JavaScriptCore/JSWebAssemblyModule.h' file not found
#include <JavaScriptCore/JSWebAssemblyModule.h>
         ^
1 error generated.
Comment 41 Don Olmstead 2017-04-14 13:46:16 PDT
This revision broke the WinCairo build. See https://build.webkit.org/builders/WinCairo%2064-Bit%20Release/builds/917/steps/compile-webkit/logs/stdio

fatal error C1083: Cannot open include file: 'JavaScriptCore/JSWebAssemblyModule.h': No such file or directory
Comment 42 Saam Barati 2017-04-14 13:54:38 PDT
Alex just landed what we think should fix the issue:
https://trac.webkit.org/changeset/215373/webkit