1885054 - Display JS trace in a debugger sidebar

Status: RESOLVED FIXED

(DevTools :: Debugger, enhancement)

Description

[Alexandre Poirot [:ochameau]]

For now the tracer can display the JavaScript tracer in:

• stdout (live)
• web console (live)
• profiler UI (only at the end of the record)

When using stdout and web console, you can open the source by clicking on the location.
It helps understand what each function call is about, especially when the code is minified.
But it can be frustrating to navigate between all frame by going back and forth between debugger and your terminal or between debugger and the web console. The Web console consume lots of space on screen and doesn't display necessarily lots of frames. Also you can navigate easily between frames via a key shortcut.

Displaying frame in a sidebar, in the debugger would help better focus on display sources, and with bug 1871500, sources with inlined values of arguments passed for each function call.
The sidebar would have a limited width, but that's ok as the function names are often small, especially on minified files.
But the sidebar would have a taller height, if you are only showing the debugger and not the split console.
And from the sidebar, focusing on display only traces, it would be easier to navigate through all the frames via a arrow keys.

Comment 1

[Alexandre Poirot [:ochameau]]

Attachment: Bug 1885054 - [devtools] Display tracer data in a debugger sidebar.

TODOs/Followups:

• probably rename CONSOLE log method to something else more generic on the server side.
  It emits JSTRACER_TRACE+JSTRACER_STATE ressources and no longer fake CONSOLE messages.
  Also the tracer actor still emits some console message in case of error or status change,
  it should be converted into tracer ressources.
• React is really bad, even when using VirtualizedTree...
• selectSourceBySourceActorID should probably be merged with panel's openSourceInDebugger
• Should the tracer panel only be visible when tracer is active? Should the tracer icon be there?
  There is also plan to move the tracer icon up at the toolbox level so that you can start it from the console (or any tool when using stdout/profiler output)...
• This getActorIdForInternalSourceId on the server side... can we pass the spidermonkey ID instead?
  It is unique per thread, but not between processes. But having to map it everytime on the server is costly.
• Show function returns
• Show arg names?

UX question:

• This ignore depth. Should we have a small left padding? Or a numeric counter? Or some smart and visualy small way to convey nested calls?
• There is tons of filtering techniques which would be neat:
  • only see tracer related to the currently selected file
  • control depth limit
  • switch to next dom event? Ctrl+DOWN?
  • show exception/error similarly to DOM events?
• Could we display a vertical stack chart here?

Comment 2

[Alexandre Poirot [:ochameau]]

Attachment: Bug 1885054 - [devtools] Allow focusing next same-depth tree item via Ctrl+Arrow keys.

Comment 3

[Alexandre Poirot [:ochameau]]

Attachment: Bug 1885054 - [devtools] Implement an inlined trace panel.

This panel allows to easily jump between each call of the currently selected function.

Comment 4

[Alexandre Poirot [:ochameau]]

Attachment: Bug 1885054 - [devtools] Cover the debugger sidebar tracer output in damp

Comment 5

[Alexandre Poirot [:ochameau]]

Attachment: Bug 1885054 - [devtools] Rename console tracer class to resources.

This will better highlight that's a generic class used to communicate live trace to devtools fronts via JSTRACE_* resources.

Comment 6

[Alexandre Poirot [:ochameau]]

Attachment: Bug 1885054 - [devtools] Pass source actor IDs instead of Debugger API Source.id.

We need that in order to have functional links for sources without URL (like evals).

Comment 7

[Alexandre Poirot [:ochameau]]

Attachment: Bug 1885054 - [devtools] Align events array with trace.

This will help use the same indexes for all the shared attributes.

Comment 8

[Alexandre Poirot [:ochameau]]

Attachment: Bug 1885054 - [devtools] Avoid mutation of the tracer frame array in the console.

The array may also be used by the debugger.

Comment 9

[Phabricator Automation]

Comment on attachment 9409983 [details]
Bug 1885054 - [devtools] Rename console tracer class to resources.

Revision D215126 was moved to bug 1906291. Setting attachment 9409983 [details] to obsolete.

Comment 10

[Phabricator Automation]

Comment on attachment 9409984 [details]
Bug 1885054 - [devtools] Pass source actor IDs instead of Debugger API Source.id.

Revision D215127 was moved to bug 1906291. Setting attachment 9409984 [details] to obsolete.

Comment 11

[Phabricator Automation]

Comment on attachment 9409985 [details]
Bug 1885054 - [devtools] Align events array with trace.

Revision D215128 was moved to bug 1906291. Setting attachment 9409985 [details] to obsolete.

Comment 12

[Phabricator Automation]

Comment on attachment 9409986 [details]
Bug 1885054 - [devtools] Avoid mutation of the tracer frame array in the console.

Revision D215129 was moved to bug 1906291. Setting attachment 9409986 [details] to obsolete.

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b6571a4222fb
[devtools] Display tracer data in a debugger sidebar. r=devtools-reviewers,nchevobbe

Comment 14

[Phabricator Automation]

Comment on attachment 9407287 [details]
Bug 1885054 - [devtools] Cover the debugger sidebar tracer output in damp

Revision D213613 was moved to bug 1908695. Setting attachment 9407287 [details] to obsolete.

Comment 15

[tszentpeteri]

https://hg.mozilla.org/mozilla-central/rev/b6571a4222fb

Comment 16

[Nicolas Chevobbe [:nchevobbe]]

Perfherder has detected a devtools performance change from push b6571a4222fb39bd9dd0e1cad22c6ff71bcc188a.

Regressions:

Ratio	Test	Platform	Options	Absolute values (old vs new)
3%	debugger-metrics	linux1804-64-qr		4,476,733.25 -> 4,613,042.33
3%	debugger-metrics	linux1804-64-tsan-qr		4,485,409.50 -> 4,621,493.67

As author of one of the patches included in that push, we need your help to address this regression.
Details of the alert can be found in the alert summary, including links to graphs and comparisons for each of the affected tests. Please follow our guide to handling regression bugs and let us know your plans within 3 business days, or the patch(es) may be backed out in accordance with our regression policy.

If you need the profiling jobs you can trigger them yourself from treeherder job view or ask a sheriff to do that for you.

You can run these tests on try with ./mach try perf --alert 1390

For more information on performance sheriffing please see our FAQ.

Comment 17

[Sandor Molnar[:smolnar]]

Backed out for causing bug 1909068

Backout link: https://hg.mozilla.org/integration/autoland/rev/4cd6f1ababe847652884c3ee36fb82ef36faca21

Comment 18

[Alexandre Poirot [:ochameau]]

The perma failure should be gone:
https://treeherder.mozilla.org/jobs?repo=try&selectedTaskRun=QYvjrH5cRHGgu15vEx0e_A.0&revision=bfe54d7ba34f7c1c3a89c527c1d0bb5f5fc3ac2f

Comment 19

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e1e9ba843c9d
[devtools] Display tracer data in a debugger sidebar. r=devtools-reviewers,nchevobbe

Comment 20

[Norisz Fay [:noriszfay]]

https://hg.mozilla.org/mozilla-central/rev/e1e9ba843c9d