Closed Bug 1358393 Opened 8 years ago Closed 7 years ago

1.64ms uninterruptible reflow at resize@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/client/webconsole/webconsole.js:661:1

Categories

(DevTools :: Console, defect, P4)

defect

Tracking

(Not tracked)

RESOLVED WORKSFORME

People

(Reporter: rjward0, Unassigned)

References

(Blocks 1 open bug)

Details

(Keywords: perf, Whiteboard: [ohnoreflow])

Here's the stack:

resize@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/client/webconsole/webconsole.js:661:1
EventListener.handleEvent*_initUI@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/client/webconsole/webconsole.js:603:5
init@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/client/webconsole/webconsole.js:477:5
WC_init@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/client/webconsole/hudservice.js:370:12
HS_openWebConsole@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/client/webconsole/hudservice.js:93:12
open/<@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/client/webconsole/panel.js:78:16
process@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:922:23
walkerLoop@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:806:7
Promise*scheduleWalkerLoop@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:739:11
schedulePromise@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:770:7
completePromise@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:707:7
open/<@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/client/webconsole/panel.js:56:9
EventListener.handleEvent*open@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/client/webconsole/panel.js:55:7
onLoad@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/client/framework/toolbox.js:1475:19
Component: Untriaged → Developer Tools: Console
The work being done in Bug 1326937 might help a bit here
See Also: → 1326937
Flags: qe-verify?
Priority: -- → P2
Marking P3 because it's not something we want to work on as part of the photon-performance project, but if the devtools team has cycles to fix it, feel free to change the priority.
Priority: P2 → P3
Whiteboard: [ohnoreflow][qf][photon-performance] → [ohnoreflow][qf][reserve-photon-performance]
Whiteboard: [ohnoreflow][qf][reserve-photon-performance] → [ohnoreflow][qf:p3][reserve-photon-performance]
Flags: qe-verify? → qe-verify-
Priority: P3 → P4
Keywords: perf
Whiteboard: [ohnoreflow][qf:p3][reserve-photon-performance] → [ohnoreflow][qf:p3][fxperf]
Code in question is:

  /**
   * Resizes the output node to fit the output wrapped.
   * We need this because it makes the layout a lot faster than
   * using -moz-box-flex and 100% width.  See Bug 1237368.
   */
  resize: function () {
    this.outputNode.style.width = this.outputWrapper.clientWidth + "px";
  },


I wonder if this could just use the new promiseLayoutFlushed helper, or rAF, or getBoundsWithoutFlushing, to avoid the sync layout fetch. :bgrins, do you remember anything from bug 1237368 that might be helpful here?

I'll mirror the qf p3 here, but arguably this should be p2/p1 because I bet it makes resizing the window with the devtools open janky. We should probably also make it a no-op when the console isn't the active tool in the devtools (but schedule an update for the next time it becomes the active tool).
Blocks: 1237368
Flags: needinfo?(bgrinstead)
Whiteboard: [ohnoreflow][qf:p3][fxperf] → [ohnoreflow][qf:p3][fxperf:p3]
(In reply to :Gijs from comment #3)
> Code in question is:
> 
>   /**
>    * Resizes the output node to fit the output wrapped.
>    * We need this because it makes the layout a lot faster than
>    * using -moz-box-flex and 100% width.  See Bug 1237368.
>    */
>   resize: function () {
>     this.outputNode.style.width = this.outputWrapper.clientWidth + "px";
>   },
> 
> 
> I wonder if this could just use the new promiseLayoutFlushed helper, or rAF,
> or getBoundsWithoutFlushing, to avoid the sync layout fetch. :bgrins, do you
> remember anything from bug 1237368 that might be helpful here?
> 
> I'll mirror the qf p3 here, but arguably this should be p2/p1 because I bet
> it makes resizing the window with the devtools open janky. We should
> probably also make it a no-op when the console isn't the active tool in the
> devtools (but schedule an update for the next time it becomes the active
> tool).

That code should only be running in the old webconsole UI which has been of by default for a while (behind the devtools.webconsole.new-frontend-enabled pref). Are you seeing jank during window resize today? Most of the issues last time I profiled were due to flexbox (bug 1410464 and bug 1408190) although https://console-html-500-logs-addition-every-1s-scroll-to-bottom.glitch.me/ does resize better for me today then it did when I filed.
Flags: needinfo?(bgrinstead) → needinfo?(gijskruitbosch+bugs)
(In reply to Brian Grinstead [:bgrins] from comment #4)
> That code should only be running in the old webconsole UI which has been of
> by default for a while (behind the devtools.webconsole.new-frontend-enabled
> pref). Are you seeing jank during window resize today?

I did not test this, I was going off the code in webconsole.js, also because `new-webconsole.js` is much smaller and talks about not being finished yet. I assumed we hadn't flipped the switch yet.

When are we removing the old thing if it's no longer being run?

> Most of the issues
> last time I profiled were due to flexbox (bug 1410464 and bug 1408190)
> although
> https://console-html-500-logs-addition-every-1s-scroll-to-bottom.glitch.me/
> does resize better for me today then it did when I filed.

OK. Should we just close this as WFM, if you're sure this is now basically dead code? :-)


(Keeping ni to see if I can reproduce the issue if I actually try!)
Flags: needinfo?(bgrinstead)
I can confirm that I only see this with the old frontend. Happily, I've found another bug (not related to devtools) through testing this, though...
Flags: needinfo?(gijskruitbosch+bugs)
Whiteboard: [ohnoreflow][qf:p3][fxperf:p3] → [ohnoreflow]
We are removing the old frontend entirely in Bug 1381834 and I've confirmed the offending line will be gone after that.
Status: NEW → RESOLVED
Closed: 7 years ago
Flags: needinfo?(bgrinstead)
Resolution: --- → WORKSFORME
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.