1338914 - Baldr: having dev console open at all causes eager wasm binary-to-text conversion

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Luke Wagner [:luke]]

If the dev console is open at all (not just Debugger), it seems that wasm::BinaryToText() is called eagerly from within a call to DebuggerSource_getText.  This should only happen when the user clicks on a wasm source in the debugger pane.

Comment 1

[Benjamin Bouvier [:bbouvier] (inactive)]

I've added a few printf to understand who was calling BinaryToText in this case. It's called by WasmCode::createText, which is called by DebuggerSourceGetTextMatcher::match, which is called by DebuggerSource_getText, which is a JS method `text` on the DebuggerSource object.

http://searchfox.org/mozilla-central/search?q=source.text&case=false&regexp=false&path=devtools%2F shows a probable subset of the calls to .text; looking at a few conditionals, it seems that:
- it is assumed that .text is very cheap, if not free. That's not the case, here...
- sometimes callers want to know if there's an actual source (by comparing source.text to a placeholder value, which is fragile), which could/should be an API provided by the debugger, maybe.

Comment 2

[Luke Wagner [:luke]]

Yury: do you know why this is being called?

Comment 3

[Yury Delendik (:yury)]

Looks like there is a check at https://dxr.mozilla.org/mozilla-central/source/devtools/server/actors/utils/TabSources.js#805:

function isHiddenSource(source) {
  // Ignore the internal Function.prototype script
  return source.text === "() {\n}";
}

by replacing it with `source.introductionType !== "wasm" && source.text === "() {\n}";` looks like problem is going anyway.

Comment 4

[Luke Wagner [:luke]]

Given that this asks for the source of every script when the dev console is open (including for profiling) and this is a slow path in general, I was wondering if the "right" fix here is to define isHiddenSource() in a way that doesn't use source.text.  Jim?

Comment 5

[Luke Wagner [:luke]]

Or, maybe, by the same logic, we don't want to be doing any of this at all unless the debugger tab is open?

Comment 6

[Yury Delendik (:yury)]

Attachment: Bug 1338914 - Optimize hidden/internal script detection for devtools.

Review commit: https://reviewboard.mozilla.org/r/114620/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/114620/

Comment 7

[Shu-yu Guo [:shu]]

Comment on attachment 8840081 [details]
Bug 1338914 - Optimize hidden/internal script detection for devtools.

https://reviewboard.mozilla.org/r/114620/#review118024

I don't like the approach for the engine to mark things as "internal" because "internal" is a subjective thing. In this particular case, it seems like the devtools frontend only cares about blacklisting Function.prototype. Could you just make the Function.prototype's source's introductionType "Function.prototype", then filter on that?

Comment 8

[Yury Delendik (:yury)]

Comment on attachment 8840081 [details]
Bug 1338914 - Optimize hidden/internal script detection for devtools.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/114620/diff/1-2/

Comment 9

[Shu-yu Guo [:shu]]

Comment on attachment 8840081 [details]
Bug 1338914 - Optimize hidden/internal script detection for devtools.

https://reviewboard.mozilla.org/r/114620/#review118526

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/8a2b1bdb1549
Optimize hidden/internal script detection for devtools. r=shu

Comment 11

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/8a2b1bdb1549