New issue
Advanced search Search tips

Issue 912043 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Dec 11
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: Bug

Blocking:
issue 912031



Sign in to add a comment

WasmSerialization needs to be isolate-independent

Project Member Reported by clemensh@chromium.org, Dec 5

Issue description

Since serialization will only receive a NativeModule, which is Isolate-independent, it should not rely on any information currently stored in the Isolate.
This basically means that WasmSerializer::isolate_ needs to disappear.
 
AFAICT it only takes the Isolate argument to get to a ExternalReferenceTable. The code that is being serialized should only contain external references that are Isolate-independent themselves. Unfortunately we don't have a reliable way to tell whether an external reference is independent of the Isolate or not. The only indication is where the factory function on ExternalReference takes the Isolate or not (i.e. see the EXTERNAL_REFERENCE_LIST_WITH_ISOLATE versus EXTERNAL_REFERENCE_LIST lists).
Yes. I will see if I can resolve that dependence.
Project Member

Comment 3 by bugdroid1@chromium.org, Dec 5

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

commit 964d17594435afb94f3f38ce4f0e27323f1b2cf8
Author: Clemens Hammacher <clemensh@chromium.org>
Date: Wed Dec 05 15:17:01 2018

Make SerializedData::kMagicNumber isolate-independent

We currently compute this value from the size of the external table,
which we get from the Isolate. This size is isolate-independent though,
so it can just be a constant.

R=mstarzinger@chromium.org

Bug:  chromium:912043 
Change-Id: If1c09a56b1a985b855f5b65818322979c194d772
Reviewed-on: https://chromium-review.googlesource.com/c/1362954
Reviewed-by: Jakob Gruber <jgruber@chromium.org>
Reviewed-by: Michael Starzinger <mstarzinger@chromium.org>
Commit-Queue: Clemens Hammacher <clemensh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#58045}
[modify] https://crrev.com/964d17594435afb94f3f38ce4f0e27323f1b2cf8/src/disassembler.cc
[modify] https://crrev.com/964d17594435afb94f3f38ce4f0e27323f1b2cf8/src/external-reference-table.h
[modify] https://crrev.com/964d17594435afb94f3f38ce4f0e27323f1b2cf8/src/isolate-data.h
[modify] https://crrev.com/964d17594435afb94f3f38ce4f0e27323f1b2cf8/src/snapshot/code-serializer.cc
[modify] https://crrev.com/964d17594435afb94f3f38ce4f0e27323f1b2cf8/src/snapshot/deserializer.cc
[modify] https://crrev.com/964d17594435afb94f3f38ce4f0e27323f1b2cf8/src/snapshot/serializer-common.cc
[modify] https://crrev.com/964d17594435afb94f3f38ce4f0e27323f1b2cf8/src/snapshot/serializer-common.h
[modify] https://crrev.com/964d17594435afb94f3f38ce4f0e27323f1b2cf8/src/snapshot/snapshot-common.cc
[modify] https://crrev.com/964d17594435afb94f3f38ce4f0e27323f1b2cf8/src/wasm/wasm-js.cc
[modify] https://crrev.com/964d17594435afb94f3f38ce4f0e27323f1b2cf8/src/wasm/wasm-serialization.cc
[modify] https://crrev.com/964d17594435afb94f3f38ce4f0e27323f1b2cf8/src/wasm/wasm-serialization.h

Project Member

Comment 4 by bugdroid1@chromium.org, 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

Status: Fixed (was: Started)

Sign in to add a comment