Issue metadata
Sign in to add a comment
|
Change parameter type of ModuleCompiledCallback |
||||||||||||||||||||||
Issue descriptionIn the context of cleaning up the relation between AsyncCompileJob and CompilationState, we will have to change the parameter type of the ModuleCompiledCallback. It currently receives a WasmCompiledModule, which corresponds to a WasmModuleObject internally (an instance of WebAssembly.Module). As such, it is isolate-specific. We need to move that callback to the CompilationState, because the AsyncCompileJob should be finished (and removed) once the module is published, i.e. if baseline compilation finished. After that, it's the CompilationState that manages tier-up and eventually calls the callback. But CompilationState is isolate-independent, so we cannot pass a specific WasmCompiledModule to the callback. Instead, we want to pass something like the TransferrableModule (a wrapper of a NativeModule). All information you need should hang off the NativeModule anyway. I would propose to 1) wait until API freeze is over (Monday) 2) move the TransferrableModule out of the WasmCompiledModule 3) add a method to retrieve the serialized bytes from the TransferrableModule 4) deprecate WasmCompiledModule::Serialize 5) change the callback to receive TransferrableModule instead of WasmCompiledModule.
,
Dec 5
The following revision refers to this bug: https://chromium.googlesource.com/v8/v8.git/+/16afa0a226ca6b2ef489117ccd3b61760b37dd85 commit 16afa0a226ca6b2ef489117ccd3b61760b37dd85 Author: Clemens Hammacher <clemensh@chromium.org> Date: Wed Dec 05 21:22:12 2018 [api][wasm] Rename WasmCompiledModule to WasmModuleObject A WasmModuleObject represents an instance of WebAssembly.Module. It is called WasmModuleObject internally, so also use that name externally. We still have a typedef for WasmCompiledModule which will be deprecated once chromium has been updated to use WasmModuleObject. R=titzer@chromium.org, adamk@chromium.org Bug: v8:8238, chromium:912031 Change-Id: I2d7708d4dc183cb4f4714f741b1ea0c153014430 Reviewed-on: https://chromium-review.googlesource.com/c/1362048 Reviewed-by: Adam Klein <adamk@chromium.org> Reviewed-by: Ben Titzer <titzer@chromium.org> Commit-Queue: Clemens Hammacher <clemensh@chromium.org> Cr-Commit-Position: refs/heads/master@{#58055} [modify] https://crrev.com/16afa0a226ca6b2ef489117ccd3b61760b37dd85/include/v8.h [modify] https://crrev.com/16afa0a226ca6b2ef489117ccd3b61760b37dd85/src/api.cc [modify] https://crrev.com/16afa0a226ca6b2ef489117ccd3b61760b37dd85/src/d8.cc [modify] https://crrev.com/16afa0a226ca6b2ef489117ccd3b61760b37dd85/src/d8.h [modify] https://crrev.com/16afa0a226ca6b2ef489117ccd3b61760b37dd85/src/runtime/runtime-test.cc [modify] https://crrev.com/16afa0a226ca6b2ef489117ccd3b61760b37dd85/src/value-serializer.cc [modify] https://crrev.com/16afa0a226ca6b2ef489117ccd3b61760b37dd85/src/wasm/wasm-js.cc [modify] https://crrev.com/16afa0a226ca6b2ef489117ccd3b61760b37dd85/test/cctest/wasm/test-wasm-codegen.cc [modify] https://crrev.com/16afa0a226ca6b2ef489117ccd3b61760b37dd85/test/cctest/wasm/test-wasm-serialization.cc [modify] https://crrev.com/16afa0a226ca6b2ef489117ccd3b61760b37dd85/test/unittests/value-serializer-unittest.cc
,
Dec 7
The following revision refers to this bug: https://chromium.googlesource.com/v8/v8.git/+/e42b547b96d80c9d95950b41be6df452736f461a commit e42b547b96d80c9d95950b41be6df452736f461a Author: Clemens Hammacher <clemensh@chromium.org> Date: Fri Dec 07 15:32:15 2018 [wasm] Serialize without accessing any Isolate We need to be able to serialize a NativeModule, which is not bound to any Isolate. Hence we should not want to pass any Isolate to the serializer. This CL removes the dependence by not using the ExternalReferenceTable from the Isolate, but instead using its own ExternalReferenceList for serialization and deserialization. This ExternalReferenceList only contains isolate-independent external references. R=mstarzinger@chromium.org Bug: chromium:912043 , chromium:912031 Change-Id: Iea5abd95dce9c54e618255cc577b6b43f002ac5d Reviewed-on: https://chromium-review.googlesource.com/c/1363135 Commit-Queue: Clemens Hammacher <clemensh@chromium.org> Reviewed-by: Michael Starzinger <mstarzinger@chromium.org> Cr-Commit-Position: refs/heads/master@{#58099} [modify] https://crrev.com/e42b547b96d80c9d95950b41be6df452736f461a/src/api.cc [modify] https://crrev.com/e42b547b96d80c9d95950b41be6df452736f461a/src/runtime/runtime-test.cc [modify] https://crrev.com/e42b547b96d80c9d95950b41be6df452736f461a/src/value-serializer.cc [modify] https://crrev.com/e42b547b96d80c9d95950b41be6df452736f461a/src/wasm/wasm-serialization.cc [modify] https://crrev.com/e42b547b96d80c9d95950b41be6df452736f461a/src/wasm/wasm-serialization.h [modify] https://crrev.com/e42b547b96d80c9d95950b41be6df452736f461a/test/cctest/wasm/test-streaming-compilation.cc
,
Dec 10
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/c9d9585eaa0e1664c000791ed6712f1521dd1dd8 commit c9d9585eaa0e1664c000791ed6712f1521dd1dd8 Author: Clemens Hammacher <clemensh@chromium.org> Date: Mon Dec 10 15:14:39 2018 Rename WasmCompiledModule to WasmModuleObject This is the blink side of the renaming in v8: https://crrev.com/c/1362048. R=mlippautz@chromium.org CC=titzer@chromium.org Bug: v8:8238, chromium:912031 Change-Id: I3877e3336c78bae456d53436a4d10c041bee92f9 Reviewed-on: https://chromium-review.googlesource.com/c/1369854 Reviewed-by: Michael Lippautz <mlippautz@chromium.org> Commit-Queue: Clemens Hammacher <clemensh@chromium.org> Cr-Commit-Position: refs/heads/master@{#615120} [modify] https://crrev.com/c9d9585eaa0e1664c000791ed6712f1521dd1dd8/third_party/blink/renderer/bindings/core/v8/serialization/serialized_script_value.h [modify] https://crrev.com/c9d9585eaa0e1664c000791ed6712f1521dd1dd8/third_party/blink/renderer/bindings/core/v8/serialization/v8_script_value_deserializer.cc [modify] https://crrev.com/c9d9585eaa0e1664c000791ed6712f1521dd1dd8/third_party/blink/renderer/bindings/core/v8/serialization/v8_script_value_deserializer.h [modify] https://crrev.com/c9d9585eaa0e1664c000791ed6712f1521dd1dd8/third_party/blink/renderer/bindings/core/v8/serialization/v8_script_value_serializer.cc [modify] https://crrev.com/c9d9585eaa0e1664c000791ed6712f1521dd1dd8/third_party/blink/renderer/bindings/core/v8/serialization/v8_script_value_serializer.h [modify] https://crrev.com/c9d9585eaa0e1664c000791ed6712f1521dd1dd8/third_party/blink/renderer/bindings/core/v8/v8_initializer.cc
,
Dec 11
The following revision refers to this bug: https://chromium.googlesource.com/v8/v8.git/+/255048c5e2721fe22922f53c2605aaa03dbed140 commit 255048c5e2721fe22922f53c2605aaa03dbed140 Author: Clemens Hammacher <clemensh@chromium.org> Date: Tue Dec 11 07:38:30 2018 [api][wasm] Change ModuleCompiledCallback definition This callback is not being used by now, so we can just change it without the deprecation dance. Instead of the WasmModuleObject, it now receives the new CompiledWasmModule wrapper which contains a shared pointer to the NativeModule. This is all that's needed for serialization. Some classes are pulled out of WasmModuleObject to allow reuse. R=adamk@chromium.org, mstarzinger@chromium.org CC=bbudge@chromium.org Bug: chromium:912031 Cq-Include-Trybots: luci.chromium.try:linux_chromium_rel_ng Change-Id: Icedb64efa92e66bec45cf8742942a07ae22f59c8 Reviewed-on: https://chromium-review.googlesource.com/c/1363140 Reviewed-by: Adam Klein <adamk@chromium.org> Reviewed-by: Bill Budge <bbudge@chromium.org> Reviewed-by: Michael Starzinger <mstarzinger@chromium.org> Commit-Queue: Clemens Hammacher <clemensh@chromium.org> Cr-Commit-Position: refs/heads/master@{#58142} [modify] https://crrev.com/255048c5e2721fe22922f53c2605aaa03dbed140/include/v8.h [modify] https://crrev.com/255048c5e2721fe22922f53c2605aaa03dbed140/src/api.cc [modify] https://crrev.com/255048c5e2721fe22922f53c2605aaa03dbed140/src/api.h [modify] https://crrev.com/255048c5e2721fe22922f53c2605aaa03dbed140/src/runtime/runtime-test.cc [modify] https://crrev.com/255048c5e2721fe22922f53c2605aaa03dbed140/src/wasm/wasm-js.cc [modify] https://crrev.com/255048c5e2721fe22922f53c2605aaa03dbed140/test/cctest/wasm/test-wasm-codegen.cc [modify] https://crrev.com/255048c5e2721fe22922f53c2605aaa03dbed140/test/cctest/wasm/test-wasm-serialization.cc
,
Dec 11
The following revision refers to this bug: https://chromium.googlesource.com/v8/v8.git/+/bffc2ab619fbf439832f3ca7e4b74adf7148bfa3 commit bffc2ab619fbf439832f3ca7e4b74adf7148bfa3 Author: Clemens Hammacher <clemensh@chromium.org> Date: Tue Dec 11 17:01:16 2018 [wasm] Move top-tier-finished callback to CompilationState The AsyncCompileJob should be decoupled from tiering, hence the top-tier-finished callback should not be delivered via the AsyncCompileJob. Instead, store it directly on the CompilationState. R=ahaas@chromium.org Bug: v8:8050, v8:7921, chromium:912031 Change-Id: Iebd64655667a8078c34caea4edeb6cf5f40833fd Reviewed-on: https://chromium-review.googlesource.com/c/1371604 Reviewed-by: Andreas Haas <ahaas@chromium.org> Commit-Queue: Clemens Hammacher <clemensh@chromium.org> Cr-Commit-Position: refs/heads/master@{#58165} [modify] https://crrev.com/bffc2ab619fbf439832f3ca7e4b74adf7148bfa3/src/wasm/compilation-environment.h [modify] https://crrev.com/bffc2ab619fbf439832f3ca7e4b74adf7148bfa3/src/wasm/module-compiler.cc [modify] https://crrev.com/bffc2ab619fbf439832f3ca7e4b74adf7148bfa3/src/wasm/streaming-decoder.cc [modify] https://crrev.com/bffc2ab619fbf439832f3ca7e4b74adf7148bfa3/src/wasm/streaming-decoder.h [modify] https://crrev.com/bffc2ab619fbf439832f3ca7e4b74adf7148bfa3/src/wasm/wasm-js.cc
,
Dec 12
The following revision refers to this bug: https://chromium.googlesource.com/v8/v8.git/+/8badfe6e9fad5a525cb845af6830d4f3d1ac2323 commit 8badfe6e9fad5a525cb845af6830d4f3d1ac2323 Author: Clemens Hammacher <clemensh@chromium.org> Date: Wed Dec 12 10:28:06 2018 [wasm][api] Deprecate WasmCompiledModule Chromium does not use this name any more since https://crrev.com/c/1369854, so we can deprecate it for the 7.3 branch. R=adamk@chromium.org Bug: v8:8238, chromium:912031 Change-Id: I0625f58a893f48d89dec76851af292c9c32af035 Cq-Include-Trybots: luci.chromium.try:linux_chromium_rel_ng Reviewed-on: https://chromium-review.googlesource.com/c/1370035 Reviewed-by: Adam Klein <adamk@chromium.org> Commit-Queue: Clemens Hammacher <clemensh@chromium.org> Cr-Commit-Position: refs/heads/master@{#58183} [modify] https://crrev.com/8badfe6e9fad5a525cb845af6830d4f3d1ac2323/include/v8.h
,
Dec 12
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/0abef58f21969131491b538ab10a5fca98bd0e35 commit 0abef58f21969131491b538ab10a5fca98bd0e35 Author: Clemens Hammacher <clemensh@chromium.org> Date: Wed Dec 12 11:58:34 2018 Avoid deprecated wasm API The API was rewritten in https://crrev.com/c/1363140. This CL changes chromium code to use the new API. R=mlippautz@chromium.org Bug: chromium:912031 Change-Id: I10493dd711b565217fcdb437bb70fa380b4cfa84 Reviewed-on: https://chromium-review.googlesource.com/c/1373749 Reviewed-by: Michael Lippautz <mlippautz@chromium.org> Commit-Queue: Clemens Hammacher <clemensh@chromium.org> Cr-Commit-Position: refs/heads/master@{#615858} [modify] https://crrev.com/0abef58f21969131491b538ab10a5fca98bd0e35/third_party/blink/renderer/bindings/core/v8/v8_initializer.cc
,
Dec 13
The following revision refers to this bug: https://chromium.googlesource.com/v8/v8.git/+/b5c757d6b79c695ab4c15de8b5b3fe3643cf1e89 commit b5c757d6b79c695ab4c15de8b5b3fe3643cf1e89 Author: Clemens Hammacher <clemensh@chromium.org> Date: Thu Dec 13 09:02:28 2018 [wasm] Deprecate old serialization API Uses of the old API were removed from chromium in https://crrev.com/c/1373749. R=adamk@chromium.org Bug: chromium:912031 Change-Id: I3fed4d72c147ef8e00ec96f869af2134e7ee71c8 Cq-Include-Trybots: luci.chromium.try:linux_chromium_rel_ng Reviewed-on: https://chromium-review.googlesource.com/c/1373769 Reviewed-by: Adam Klein <adamk@chromium.org> Commit-Queue: Clemens Hammacher <clemensh@chromium.org> Cr-Commit-Position: refs/heads/master@{#58205} [modify] https://crrev.com/b5c757d6b79c695ab4c15de8b5b3fe3643cf1e89/include/v8.h
,
Dec 13
Fixed for now. We still need to remove the deprecated API after the M-73 branch. |
|||||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||||
Comment 1 by clemensh@chromium.org
, Dec 5