New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 667767 link

Starred by 1 user

Issue metadata

Status: Assigned
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Feature



Sign in to add a comment

Wasm: Don't allocate Script, assign script id directly for compiled modules

Project Member Reported by clemensh@chromium.org, Nov 22 2016

Issue description

I discussed this with Jakob. The Script with type TYPE_WASM is currently only a wrapper for the WasmCompiledModule. We don't use any of the existing functionality, but always just redirect to wasm functions from Script functions.

The idea is to just not allocate the Script at all, but assign a script id directly to a WasmCompiledModule, and move the disambiguation between Script and WasmCompiledModule to the debug methods which get a script id and call the respective methods.
 
Cc: kozyatinskiy@chromium.org
One way to implement this is to make DebugInterface::Script wrap either an i::Script or an i::WasmCompiledModule. This would move all distinctions (wasm / non-wasm) from i::Script to DebugInterface::Script.

IMO this nicely corresponds to the fact that to the debugger, both will be represented by a script id.

WDYT?
Project Member

Comment 2 by bugdroid1@chromium.org, Dec 6 2016

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

commit a9017cb0184ec5f1f42b74e26aa1055b9ea6633b
Author: clemensh <clemensh@chromium.org>
Date: Tue Dec 06 14:26:15 2016

[inspector] Split V8DebuggerScript implementation for wasm

Make some methods on V8DebuggerScript virtual and provide the
implementations ActualScript for scripts which are backed by scripts on
V8's side, and WasmVirtualScript for wasm scripts.

The added test case ensures that we at least don't crash on the attempt
to get breakable locations for wasm "scripts", which we did previously.
Returning a reasonable result for wasm will be implemented in a
follow-up commit.

R=yangguo@chromium.org, jgruber@chromium.org
BUG=chromium:667767, chromium:613110 

Review-Url: https://codereview.chromium.org/2532433003
Cr-Commit-Position: refs/heads/master@{#41527}

[modify] https://crrev.com/a9017cb0184ec5f1f42b74e26aa1055b9ea6633b/src/inspector/v8-debugger-script.cc
[modify] https://crrev.com/a9017cb0184ec5f1f42b74e26aa1055b9ea6633b/src/inspector/v8-debugger-script.h
[modify] https://crrev.com/a9017cb0184ec5f1f42b74e26aa1055b9ea6633b/src/inspector/v8-debugger.cc
[modify] https://crrev.com/a9017cb0184ec5f1f42b74e26aa1055b9ea6633b/src/inspector/wasm-translation.cc

Sign in to add a comment