[wasm] Copy the signature when compiling an imported function.

[wasm] Copy the signature when compiling an imported function.

The signature of an imported function is needed to compile a wrapper in
wasm to call the imported function. The signature is stored in a heap
object which is created when the wasm module is compiled. With this CL
we do not use a pointer to the signature in the heap object but instead
copy the signature and then use a pointer to the copy. A pointer into
a heap object causes problems when a GC is happening.

R=titzer@chromium.org, mtrofin@chromium.org

Committed: https://crrev.com/65415ca79584506c6ce2d78139a7dda608f8978e
Cr-Commit-Position: refs/heads/master@{#37527}

[titzer, 2016-07-05 12:13:00]

lgtm

[ahaas, 2016-07-05 12:13:29]

The CQ bit was checked by ahaas@chromium.org

[commit-bot: I haz the power, 2016-07-05 12:13:34]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-07-05 12:15:21]

Committed patchset #1 (id:1)

[commit-bot: I haz the power, 2016-07-05 12:17:12]

Description was changed from

==========
[wasm] Copy the signature when compiling an imported function.

The signature of an imported function is needed to compile a wrapper in
wasm to call the imported function. The signature is stored in a heap
object which is created when the wasm module is compiled. With this CL
we do not use a pointer to the signature in the heap object but instead
copy the signature and then use a pointer to the copy. A pointer into
a heap object causes problems when a GC is happening.

R=titzer@chromium.org, mtrofin@chromium.org
==========

to

==========
[wasm] Copy the signature when compiling an imported function.

The signature of an imported function is needed to compile a wrapper in
wasm to call the imported function. The signature is stored in a heap
object which is created when the wasm module is compiled. With this CL
we do not use a pointer to the signature in the heap object but instead
copy the signature and then use a pointer to the copy. A pointer into
a heap object causes problems when a GC is happening.

R=titzer@chromium.org, mtrofin@chromium.org

Committed: https://crrev.com/65415ca79584506c6ce2d78139a7dda608f8978e
Cr-Commit-Position: refs/heads/master@{#37527}
==========

[commit-bot: I haz the power, 2016-07-05 12:17:13]

Patchset 1 (id:??) landed as
https://crrev.com/65415ca79584506c6ce2d78139a7dda608f8978e
Cr-Commit-Position: refs/heads/master@{#37527}

[Mircea Trofin, 2016-07-05 15:05:43]

On 2016/07/05 12:17:13, commit-bot: I haz the power wrote:
> Patchset 1 (id:??) landed as
> https://crrev.com/65415ca79584506c6ce2d78139a7dda608f8978e
> Cr-Commit-Position: refs/heads/master@{#37527}

Lgtm

[Mircea Trofin, 2016-07-05 16:02:03]

On 2016/07/05 15:05:43, Mircea Trofin wrote:
> On 2016/07/05 12:17:13, commit-bot: I haz the power wrote:
> > Patchset 1 (id:??) landed as
> > https://crrev.com/65415ca79584506c6ce2d78139a7dda608f8978e
> > Cr-Commit-Position: refs/heads/master@{#37527}
> 
> Lgtm

FWIW, I believe the only time we're doing a heap allocation there is when we
construct the code object, at 
which time we don't need the signature object anymore. Have you seen a failure
somewhere? Even if not, I 
like your CL because it improves robustness.

Now, if the above is correct and we only allocate on the JS heap as described,
we could factor
CompileWasmToJSWrapper into the part that builds up the graph (and needs the
signature) and the part that
codegens (and allocates on the JS heap), and we could then avoid allocating and
copying yet again.

[ahaas, 2016-07-05 17:06:11]

On 2016/07/05 at 16:02:03, mtrofin wrote:
> On 2016/07/05 15:05:43, Mircea Trofin wrote:
> > On 2016/07/05 12:17:13, commit-bot: I haz the power wrote:
> > > Patchset 1 (id:??) landed as
> > > https://crrev.com/65415ca79584506c6ce2d78139a7dda608f8978e
> > > Cr-Commit-Position: refs/heads/master@{#37527}
> > 
> > Lgtm
> 
> FWIW, I believe the only time we're doing a heap allocation there is when we
construct the code object, at 
> which time we don't need the signature object anymore. Have you seen a failure
somewhere? Even if not, I 
> like your CL because it improves robustness.
> 
Some tests crashed sometimes on the stress-gc bots because the pointer to the
signature array was invalid.

There is some heap allocation happening during the graph construction of the
WasmToJSWrapper: The wrapper uses the {AllocateHeapNumber} code stub to prepare
float parameters for javascript. The code of this code stub is generated when it
is used for the first time, which can also be during the graph construction of
the wrapper. This problem with code stubs is already on my list, because it
blocks my attempt to extend parallel compilation to most parts of code
generation.

> Now, if the above is correct and we only allocate on the JS heap as described,
we could factor
> CompileWasmToJSWrapper into the part that builds up the graph (and needs the
signature) and the part that
> codegens (and allocates on the JS heap), and we could then avoid allocating
and copying yet again.

We could even think about adding the wasm-to-js wrappers to parallel compilation
if we manage to avoid all heap allocation.

[Mircea Trofin, 2016-07-06 16:26:24]

On 2016/07/05 17:06:11, ahaas wrote:
> On 2016/07/05 at 16:02:03, mtrofin wrote:
> > On 2016/07/05 15:05:43, Mircea Trofin wrote:
> > > On 2016/07/05 12:17:13, commit-bot: I haz the power wrote:
> > > > Patchset 1 (id:??) landed as
> > > > https://crrev.com/65415ca79584506c6ce2d78139a7dda608f8978e
> > > > Cr-Commit-Position: refs/heads/master@{#37527}
> > > 
> > > Lgtm
> > 
> > FWIW, I believe the only time we're doing a heap allocation there is when we
> construct the code object, at 
> > which time we don't need the signature object anymore. Have you seen a
failure
> somewhere? Even if not, I 
> > like your CL because it improves robustness.
> > 
> Some tests crashed sometimes on the stress-gc bots because the pointer to the
> signature array was invalid.
> 
> There is some heap allocation happening during the graph construction of the
> WasmToJSWrapper: The wrapper uses the {AllocateHeapNumber} code stub to
prepare
> float parameters for javascript. The code of this code stub is generated when
it
> is used for the first time, which can also be during the graph construction of
> the wrapper. This problem with code stubs is already on my list, because it
> blocks my attempt to extend parallel compilation to most parts of code
> generation.
> 
> > Now, if the above is correct and we only allocate on the JS heap as
described,
> we could factor
> > CompileWasmToJSWrapper into the part that builds up the graph (and needs the
> signature) and the part that
> > codegens (and allocates on the JS heap), and we could then avoid allocating
> and copying yet again.
> 
Thanks for the details!

> We could even think about adding the wasm-to-js wrappers to parallel
compilation
> if we manage to avoid all heap allocation.
Nice - we'd still need to defer that to instantiation time, though. 

BTW, do we have an idea how much compiling imports, and, separately, exports
costs,
vs the total compile time?