1654590 - Remove observeAsmJS thread option

Status: ASSIGNED

(DevTools :: Debugger, task, P3)

Description

[Julian Descottes [:jdescottes]]

Follow up to Bug 1633727.

The thread option observeAsmJS is always set to true.
We should be able to remove it https://searchfox.org/mozilla-central/search?q=observeAsmJS&path=devtools

Comment 1

[Nicolas Chevobbe [:nchevobbe]]

Adding pauseWorkersUntilAttach into the mix. It's only used in reconfigure and the client always sends "true".
For now I'll just remove the check on this option and always call the code that used to be called, and I'll take care of any cleanup we might want to do around that in Bug 1633712

Comment 2

[Nicolas Chevobbe [:nchevobbe]]

Attachment: Bug 1654590 - Remove observeAsmJS and pauseWorkersUntilAttach thread option. r=jdescottes.

The options were always set to true so it's safe to remove them.

Comment 3

[Lars T Hansen [:lth]]

It would appear that this bug is the source of some real problems we're seeing with the web console, see bug 1714072, bug 1714086. Enabling asm.js debugging just for the console causes us to generate slow code and massive amounts of metadata for wasm. Together with the console storage leak over in bug 1476869 the result is that even printf debugging works poorly for wasm.

Can a fix for the present bug be expedited?

Comment 4

[Nicolas Chevobbe [:nchevobbe]]

(In reply to Lars T Hansen [:lth] from comment #3)

It would appear that this bug is the source of some real problems we're seeing with the web console, see bug 1714072, bug 1714086. Enabling asm.js debugging just for the console causes us to generate slow code and massive amounts of metadata for wasm. Together with the console storage leak over in bug 1476869 the result is that even printf debugging works poorly for wasm.

Can a fix for the present bug be expedited?

This specific bug wouldn't impact the performance issue; it's only about removing the ability to set the option, as it always end up being true.
But with what you're saying, maybe we should keep the option around, and have a way to expose it in devtools UI.
I guess why this is impacting performance is described here: https://searchfox.org/mozilla-central/rev/b355f4e36eb45ef84ce9d3dd6af7f1a417ca8bfe/js/src/doc/Debugger/Debugger.md#16-22 (i.e. we have to set it so you can pause and steps in asm). So we could have an "asm debugger support" option (or something better worded), that would warn about performance cost.
We could also be smarter and only set the allowUnobservedAsmJS only if the debugger is open, or only when the user tries to set a breakpoint in an asm.js file.

Comment 5

[Lars T Hansen [:lth]]

Thanks, that's helpful. Setting allowUnobservedAsmJS only when the debugger is open would be a good compromise IMO, no further UI should be required. We have a job to do on our side with reducing the volume of debug data, but having it generated only when debugging is plausibly going to happen would help considerably with the current papercut.

We probably should also consider (a little later) whether there's a difference between asm.js and wasm as to which policies are most appropriate; for the moment, we have allowed the asm.js settings to be applied to wasm code too. That said, I expect asm.js code also does not need to be pessimized when the console is open, only when debugging. (And asm.js is dying, we're just waiting for the right excuse to rip out the dedicated asm.js jit.)