1355869 - At startup scrollToBottom call takes a good chunk of time

Status: RESOLVED FIXED

(DevTools :: Console, enhancement, P1)

Description

[Nicolas Chevobbe [:nchevobbe]]

In this profile https://perfht.ml/2o5lA6n there were 2000 messages logged.
The scrollToBottom call takes ~300ms when the whole console init is ~2500ms. I saw this took even longer when the components have a bigger height.

If the call is delayed to the nextTick (with setTimeout), the call takes ~60ms (https://perfht.ml/2o5oSq8).

It would be nice to see if there is any drawbacks of doing this because it looks like an easy win

Comment 1

[Nicolas Chevobbe [:nchevobbe]]

Here's the scrollToBottom call i'm talking about http://searchfox.org/mozilla-central/source/devtools/client/webconsole/new-console-output/components/console-output.js#42

Comment 2

[Brian Grinstead [:bgrins]]

I'd be happy with the improvement as long as it doesn't cause an incorrect measurement on isScrolledToBottom (which would cause the console to become 'unpinned').

Comment 3

[Nicolas Chevobbe [:nchevobbe]]

Attachment: Bug 1355869 - Delay scrollToBottom on next tick in console init.

The call to scrollToBottom could take a good amount of time if there were
already logged messages in the console. This might be related to the fact that
componentDidMount is called although there is some layout or paint work.
Delaying the first call to scrollToBottom in the component greatly reduced the
time spent in scrollToBottom.

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

Comment 4

[Nicolas Chevobbe [:nchevobbe]]

(In reply to Brian Grinstead [:bgrins] from comment #2)
> I'd be happy with the improvement as long as it doesn't cause an incorrect
> measurement on isScrolledToBottom (which would cause the console to become
> 'unpinned').

I tested it with your stress page (https://bgrins.github.io/devtools-demos/console/stress.html), and everything seems to work fine, since I'm only doing it on the componentDidMount.

Comment 5

[Nicolas Chevobbe [:nchevobbe]]

TRY is here https://treeherder.mozilla.org/#/jobs?repo=try&revision=bd778256058f8737df9fcc8a79eb16e4ac015f9e

Comment 6

[Brian Grinstead [:bgrins]]

Comment on attachment 8857862 [details]
Bug 1355869 - Delay scrollToBottom on next tick in console init.

https://reviewboard.mozilla.org/r/129490/#review132612

I guess there is a potential race here where a message comes in before the initial scrollToBottom is called, which then fails a check of isScrolledToBottom, but it would be scrolled ultimately anyway once the timeout fires.  Looks like a nice perf improvement for minimal change.

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a8a257ba1903
Delay scrollToBottom on next tick in console init. r=bgrins

Comment 8

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

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