New issue
Advanced search Search tips

Issue 860637 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Aug 27
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

Tracking bug for the refactoring of WebAssembly.instantiateStreaming

Project Member Reported by ahaas@chromium.org, Jul 6

Issue description

The implementation of WebAssembly.instantiateStreaming is not spec-compliant at the moment. This issue is the tracking bug to refactor the implementation and make it spec compliant again.

The design doc for this change is available at: http://doc/1VMDM5hiVyKpHTfAj85AnKhuV8AdvEiFUIe6ii9pgo8U

The prototype implementation:
V8: https://chromium-review.googlesource.com/c/v8/v8/+/1080821
Chrome: https://chromium-review.googlesource.com/c/chromium/src/+/1081947
 
Project Member

Comment 1 by bugdroid1@chromium.org, Jul 12

The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/7971f9d6f747708b2847aa5a485cbff1f7a446ea

commit 7971f9d6f747708b2847aa5a485cbff1f7a446ea
Author: Andreas Haas <ahaas@chromium.org>
Date: Thu Jul 12 09:06:28 2018

[wasm] Prepare API changes for WebAssembly.instantiateStreaming

This is the first CL to refactor WebAssembly.instantiateStreaming to
make it spec compliant again. The design doc where the whole change is
discussed is available in the tracking bug. The tracking bug also
references prototype implementations of the whole change, which includes
the changes in this CL.

Tests for the new API functions will be added with the second V8 CL
which adds the implementation of the API functions.

R=ulan@chromium.org

Bug:  chromium:860637 
Cq-Include-Trybots: luci.chromium.try:linux_chromium_rel_ng
Change-Id: Ia1048b7ca0291c824ef4212ebde2c6054e102069
Reviewed-on: https://chromium-review.googlesource.com/1127667
Reviewed-by: Ulan Degenbaev <ulan@chromium.org>
Commit-Queue: Andreas Haas <ahaas@chromium.org>
Cr-Commit-Position: refs/heads/master@{#54393}
[modify] https://crrev.com/7971f9d6f747708b2847aa5a485cbff1f7a446ea/include/v8.h
[modify] https://crrev.com/7971f9d6f747708b2847aa5a485cbff1f7a446ea/src/api.cc
[modify] https://crrev.com/7971f9d6f747708b2847aa5a485cbff1f7a446ea/src/isolate.h
[modify] https://crrev.com/7971f9d6f747708b2847aa5a485cbff1f7a446ea/src/wasm/wasm-js.cc

Project Member

Comment 2 by bugdroid1@chromium.org, Jul 24

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/2232eb7e1088862796cd62b9a6078131039bf62a

commit 2232eb7e1088862796cd62b9a6078131039bf62a
Author: Andreas Haas <ahaas@chromium.org>
Date: Tue Jul 24 16:57:51 2018

[v8][wasm] Replace desugaring of WebAssembly.instantiateStreaming

This is the blink side CL to refactor WebAssembly.instantiateStreaming
to make it spec compliant again. The design doc where the whole change
is discussed is available in the tracking bug. The tracking bug also
references prototype implementations of the whole change, which includes
the changes in this CL.

Note that most of the changes in this CL are copied from the existing
implementation which uses the old API.

I added a regression test but disable it until the V8 change landed.

Bug:  chromium:860637 

Change-Id: Iba75dc4e187aad5891084e5e7dcc3f759e86f706
Reviewed-on: https://chromium-review.googlesource.com/1081947
Commit-Queue: Andreas Haas <ahaas@chromium.org>
Reviewed-by: Yuki Shiino <yukishiino@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Cr-Commit-Position: refs/heads/master@{#577586}
[modify] https://crrev.com/2232eb7e1088862796cd62b9a6078131039bf62a/third_party/WebKit/LayoutTests/TestExpectations
[add] https://crrev.com/2232eb7e1088862796cd62b9a6078131039bf62a/third_party/WebKit/LayoutTests/http/tests/wasm_streaming/regression860637.html
[modify] https://crrev.com/2232eb7e1088862796cd62b9a6078131039bf62a/third_party/WebKit/LayoutTests/http/tests/wasm_streaming/wasm_response_apis.js
[modify] https://crrev.com/2232eb7e1088862796cd62b9a6078131039bf62a/third_party/blink/renderer/bindings/core/v8/v8_wasm_response_extensions.cc

Project Member

Comment 3 by bugdroid1@chromium.org, Aug 1

The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/b556c9eaa6acbe14be8e63e168f0ae5f0ab23e35

commit b556c9eaa6acbe14be8e63e168f0ae5f0ab23e35
Author: Andreas Haas <ahaas@chromium.org>
Date: Wed Aug 01 08:56:21 2018

[wasm] Implement the new API for WebAssembly.instantiateStreaming

This is the second V8 CL to refactor WebAssembly.instantiateStreaming to
make it spec compliant again. The design doc where the whole change is
discussed is available in the tracking bug. The tracking bug also
references prototype implementations of the whole change, which includes
the changes in this CL.

R=mstarzinger@chromium.org

Bug:  chromium:860637 
Cq-Include-Trybots: luci.chromium.try:linux_chromium_rel_ng
Change-Id: I776c0f24959ab5663727d3dfee0248a9b0642a42
Reviewed-on: https://chromium-review.googlesource.com/1143187
Commit-Queue: Andreas Haas <ahaas@chromium.org>
Reviewed-by: Michael Starzinger <mstarzinger@chromium.org>
Cr-Commit-Position: refs/heads/master@{#54834}
[modify] https://crrev.com/b556c9eaa6acbe14be8e63e168f0ae5f0ab23e35/src/api.cc
[modify] https://crrev.com/b556c9eaa6acbe14be8e63e168f0ae5f0ab23e35/src/wasm/module-compiler.cc
[modify] https://crrev.com/b556c9eaa6acbe14be8e63e168f0ae5f0ab23e35/src/wasm/module-compiler.h
[modify] https://crrev.com/b556c9eaa6acbe14be8e63e168f0ae5f0ab23e35/src/wasm/streaming-decoder.cc
[modify] https://crrev.com/b556c9eaa6acbe14be8e63e168f0ae5f0ab23e35/src/wasm/wasm-engine.cc
[modify] https://crrev.com/b556c9eaa6acbe14be8e63e168f0ae5f0ab23e35/src/wasm/wasm-engine.h
[modify] https://crrev.com/b556c9eaa6acbe14be8e63e168f0ae5f0ab23e35/src/wasm/wasm-js.cc
[modify] https://crrev.com/b556c9eaa6acbe14be8e63e168f0ae5f0ab23e35/test/cctest/test-api.cc
[modify] https://crrev.com/b556c9eaa6acbe14be8e63e168f0ae5f0ab23e35/test/cctest/wasm/test-streaming-compilation.cc
[modify] https://crrev.com/b556c9eaa6acbe14be8e63e168f0ae5f0ab23e35/test/fuzzer/wasm-async.cc

Project Member

Comment 4 by bugdroid1@chromium.org, Aug 2

The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/fea9300d9fef0193b57194abac4fb12e03676695

commit fea9300d9fef0193b57194abac4fb12e03676695
Author: Andreas Haas <ahaas@chromium.org>
Date: Thu Aug 02 13:29:47 2018

Revert "[wasm] Implement the new API for WebAssembly.instantiateStreaming"

This reverts commit b556c9eaa6acbe14be8e63e168f0ae5f0ab23e35.

Reason for revert: Flakes in layout tests:  https://crbug.com/870187 

Original change's description:
> [wasm] Implement the new API for WebAssembly.instantiateStreaming
> 
> This is the second V8 CL to refactor WebAssembly.instantiateStreaming to
> make it spec compliant again. The design doc where the whole change is
> discussed is available in the tracking bug. The tracking bug also
> references prototype implementations of the whole change, which includes
> the changes in this CL.
> 
> R=​mstarzinger@chromium.org
> 
> Bug:  chromium:860637 
> Cq-Include-Trybots: luci.chromium.try:linux_chromium_rel_ng
> Change-Id: I776c0f24959ab5663727d3dfee0248a9b0642a42
> Reviewed-on: https://chromium-review.googlesource.com/1143187
> Commit-Queue: Andreas Haas <ahaas@chromium.org>
> Reviewed-by: Michael Starzinger <mstarzinger@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#54834}

TBR=mstarzinger@chromium.org,ahaas@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug:  chromium:860637 
Change-Id: Icbf2603143068a49c61de162aa7185a753703e5d
Cq-Include-Trybots: luci.chromium.try:linux_chromium_rel_ng
Reviewed-on: https://chromium-review.googlesource.com/1160261
Reviewed-by: Michael Starzinger <mstarzinger@chromium.org>
Reviewed-by: Andreas Haas <ahaas@chromium.org>
Commit-Queue: Andreas Haas <ahaas@chromium.org>
Cr-Commit-Position: refs/heads/master@{#54872}
[modify] https://crrev.com/fea9300d9fef0193b57194abac4fb12e03676695/src/api.cc
[modify] https://crrev.com/fea9300d9fef0193b57194abac4fb12e03676695/src/wasm/module-compiler.cc
[modify] https://crrev.com/fea9300d9fef0193b57194abac4fb12e03676695/src/wasm/module-compiler.h
[modify] https://crrev.com/fea9300d9fef0193b57194abac4fb12e03676695/src/wasm/streaming-decoder.cc
[modify] https://crrev.com/fea9300d9fef0193b57194abac4fb12e03676695/src/wasm/wasm-engine.cc
[modify] https://crrev.com/fea9300d9fef0193b57194abac4fb12e03676695/src/wasm/wasm-engine.h
[modify] https://crrev.com/fea9300d9fef0193b57194abac4fb12e03676695/src/wasm/wasm-js.cc
[modify] https://crrev.com/fea9300d9fef0193b57194abac4fb12e03676695/test/cctest/test-api.cc
[modify] https://crrev.com/fea9300d9fef0193b57194abac4fb12e03676695/test/cctest/wasm/test-streaming-compilation.cc
[modify] https://crrev.com/fea9300d9fef0193b57194abac4fb12e03676695/test/fuzzer/wasm-async.cc

Project Member

Comment 5 by bugdroid1@chromium.org, Aug 13

The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/df18e841400be3730fc3e2aec03d13781afc0b41

commit df18e841400be3730fc3e2aec03d13781afc0b41
Author: Andreas Haas <ahaas@chromium.org>
Date: Mon Aug 13 13:43:20 2018

[wasm] Create HandleScope in OnFinishedStream

{AsyncCompileJob::FinishCompile} assumes that it is called within a
{HandleScope}. This was not the case when it was called at the end of
streaming compilation.

R=clemensh@chromium.org

Bug:  chromium:860637 
Change-Id: I74508e6cdfc145efb9adc76176abce1ca5713515
Reviewed-on: https://chromium-review.googlesource.com/1172357
Reviewed-by: Clemens Hammacher <clemensh@chromium.org>
Commit-Queue: Andreas Haas <ahaas@chromium.org>
Cr-Commit-Position: refs/heads/master@{#55086}
[modify] https://crrev.com/df18e841400be3730fc3e2aec03d13781afc0b41/src/wasm/module-compiler.cc

Project Member

Comment 6 by bugdroid1@chromium.org, Aug 14

The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/3e545e404525540ada1c3ce3aacaab902150324e

commit 3e545e404525540ada1c3ce3aacaab902150324e
Author: Andreas Haas <ahaas@chromium.org>
Date: Tue Aug 14 08:20:19 2018

Reland "[wasm] Implement the new API for WebAssembly.instantiateStreaming"

The problem was that in AsyncCompileJob::FinishModule we allocate a
handle, but when this function is called from streaming compilation, then
there was no HandleScope around AsyncCompileJob::FinishModule. This issue
was fixed in another CL, https://crrev.com/c/1172357. This CL is just a
rebase of the original CL.

Original change's description:
> [wasm] Implement the new API for WebAssembly.instantiateStreaming

> This is the second V8 CL to refactor WebAssembly.instantiateStreaming to
> make it spec compliant again. The design doc where the whole change is
> discussed is available in the tracking bug. The tracking bug also
> references prototype implementations of the whole change, which includes
> the changes in this CL.

R=starzinger@chromium.org

Bug:  chromium:860637 
Cq-Include-Trybots: luci.chromium.try:linux_chromium_rel_ng
Change-Id: Ib0cb25488654d2b325b4f529d33b76b846c64436
Reviewed-on: https://chromium-review.googlesource.com/1172429
Reviewed-by: Clemens Hammacher <clemensh@chromium.org>
Commit-Queue: Andreas Haas <ahaas@chromium.org>
Cr-Commit-Position: refs/heads/master@{#55106}
[modify] https://crrev.com/3e545e404525540ada1c3ce3aacaab902150324e/src/api.cc
[modify] https://crrev.com/3e545e404525540ada1c3ce3aacaab902150324e/src/wasm/module-compiler.cc
[modify] https://crrev.com/3e545e404525540ada1c3ce3aacaab902150324e/src/wasm/module-compiler.h
[modify] https://crrev.com/3e545e404525540ada1c3ce3aacaab902150324e/src/wasm/streaming-decoder.cc
[modify] https://crrev.com/3e545e404525540ada1c3ce3aacaab902150324e/src/wasm/wasm-engine.cc
[modify] https://crrev.com/3e545e404525540ada1c3ce3aacaab902150324e/src/wasm/wasm-engine.h
[modify] https://crrev.com/3e545e404525540ada1c3ce3aacaab902150324e/src/wasm/wasm-js.cc
[modify] https://crrev.com/3e545e404525540ada1c3ce3aacaab902150324e/test/cctest/test-api.cc
[modify] https://crrev.com/3e545e404525540ada1c3ce3aacaab902150324e/test/cctest/wasm/test-streaming-compilation.cc
[modify] https://crrev.com/3e545e404525540ada1c3ce3aacaab902150324e/test/fuzzer/wasm-async.cc

Status: Fixed (was: Started)
Project Member

Comment 8 by bugdroid1@chromium.org, Sep 6

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ca58a477fe701fac2dcf129c534c8bb2b8f30172

commit ca58a477fe701fac2dcf129c534c8bb2b8f30172
Author: Andreas Haas <ahaas@chromium.org>
Date: Thu Sep 06 13:57:17 2018

[v8][wasm] Re-enable wasm-streaming tests

This CL re-enables regression tests for
WebAssembly.instantiateStreaming. See the bug link for the fixes
themselves.

R=haraken@chromium.org

Bug:  chromium:860637 
Change-Id: I0aee6004c524f4e81adf68891d78341bacbc2237
Reviewed-on: https://chromium-review.googlesource.com/1209742
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Commit-Queue: Andreas Haas <ahaas@chromium.org>
Cr-Commit-Position: refs/heads/master@{#589152}
[modify] https://crrev.com/ca58a477fe701fac2dcf129c534c8bb2b8f30172/third_party/WebKit/LayoutTests/TestExpectations

Project Member

Comment 9 by bugdroid1@chromium.org, Nov 22

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/d92a61445ac02302557ce52c742e7f11e52646b9

commit d92a61445ac02302557ce52c742e7f11e52646b9
Author: Andreas Haas <ahaas@chromium.org>
Date: Thu Nov 22 10:40:32 2018

[v8][wasm] Delete old streaming compilation implementation

The new implementation landed already some time ago and is stable, so
we can delete the old implementation now.

R=mlippautz@chromium.org

Bug:  chromium:860637 
Change-Id: I3cffd1ef0f879d144c5705618165ca52542f3e15
Reviewed-on: https://chromium-review.googlesource.com/c/1345969
Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
Commit-Queue: Andreas Haas <ahaas@chromium.org>
Cr-Commit-Position: refs/heads/master@{#610367}
[modify] https://crrev.com/d92a61445ac02302557ce52c742e7f11e52646b9/third_party/blink/renderer/bindings/core/v8/v8_wasm_response_extensions.cc

Project Member

Comment 10 by bugdroid1@chromium.org, Nov 23

The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/1624b5c679014c6454080207f2c21cc577b5d584

commit 1624b5c679014c6454080207f2c21cc577b5d584
Author: Andreas Haas <ahaas@chromium.org>
Date: Fri Nov 23 12:51:02 2018

[api][wasm] Mark all streaming compilation callback as DEPRECATE_SOON

The callback set with this function is already not used anymore.

R=yangguo@chromium.org

Bug:  chromium:860637 , v8:8238
Change-Id: I26f4528720e936dcc9b7b244dff7db97a4b43273
Reviewed-on: https://chromium-review.googlesource.com/c/1345989
Reviewed-by: Yang Guo <yangguo@chromium.org>
Commit-Queue: Andreas Haas <ahaas@chromium.org>
Cr-Commit-Position: refs/heads/master@{#57783}
[modify] https://crrev.com/1624b5c679014c6454080207f2c21cc577b5d584/include/v8.h

Sign in to add a comment