1160332 - Reselecting with calltree in view results in choppy rerendering

Status: RESOLVED FIXED

(DevTools :: Performance Tools (Profiler/Timeline), defect, P1)

Description

[Jordan Santell [:jsantell] [@jsantell]]

Suspect a lazy debounce or lazy rendering of the calltree will fix this. Gif: http://i.imgur.com/zR2eAVJ.gif

This is important because in 40.0, we will ONLY have the js-calltree with the framerate graph.

Comment 1

[Victor Porof [:vporof][:vp]]

(In reply to Jordan Santell [:jsantell] [@jsantell] from comment #0)
> Suspect a lazy debounce or lazy rendering of the calltree will fix this.
> Gif: http://i.imgur.com/zR2eAVJ.gif
> 

We already have a debounce, but it's probably not enough. Also, Nick's new `shouldUpdateWhileMouseIsActive` from bug 1157914 should help a lot.

Comment 2

[Jordan Santell [:jsantell] [@jsantell]]

Attachment: 1160332-mouseactive.patch

In the bugzilla flurry, did not realize you grabbed this, victor. It looks like the patch in bug 1157914 was never rebased after the GraphsController landed, so it didn't actually work. This goes on top of it, fixing the jank for non flamegraph details views
https://treeherder.mozilla.org/#/jobs?repo=try&revision=766243eb6833

Comment 3

[Victor Porof [:vporof][:vp]]

Comment on attachment 8600190 [details] [diff] [review]
1160332-mouseactive.patch

Review of attachment 8600190 [details] [diff] [review]:
-----------------------------------------------------------------

comments

::: browser/devtools/performance/modules/graphs.js
@@ +328,5 @@
>    /**
> +   * Returns an array of graphs that have been created, not necessarily
> +   * enabled currently.
> +   */
> +  getGraphs: function () {

getWidgets() is probably better. graphs.getGraphs() is weird as hell.

::: browser/devtools/performance/views/overview.js
@@ +99,5 @@
>    get isMouseActive() {
> +    // Fetch all graphs currently stored in the GraphsController.
> +    // These graphs are not necessarily active, but will not have
> +    // an active mouse, in that case.
> +    for (let graph of this.graphs.getGraphs()) {

return !!this.graphs.getGraphs().find(e => e.isMouseActive); is nicer.

Comment 4

[Victor Porof [:vporof][:vp]]

Comment on attachment 8600190 [details] [diff] [review]
1160332-mouseactive.patch

Review of attachment 8600190 [details] [diff] [review]:
-----------------------------------------------------------------

Actually, I'd love to see a small test for this.

Comment 5

[Jordan Santell [:jsantell] [@jsantell]]

Attachment: 1160332-mouseactive.patch

https://treeherder.mozilla.org/#/jobs?repo=try&revision=999a76f1b86b

Comment 6

[Pulsebot]

https://hg.mozilla.org/integration/fx-team/rev/14c8c3fd403c

Comment 7

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/14c8c3fd403c