At startup scrollToBottom call takes a good chunk of time

RESOLVED FIXED in Firefox 55

Status

P1
normal
RESOLVED FIXED
2 years ago
6 months ago

People

(Reporter: nchevobbe, Assigned: nchevobbe)

Tracking

unspecified
Firefox 55
Bug Flags:
qe-verify -

Firefox Tracking Flags

(firefox55 fixed)

Details

(Whiteboard: [console-html])

Attachments

(1 attachment)

(Assignee)

Description

2 years ago
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
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').
Flags: qe-verify?
Whiteboard: [new-console] [triage]
Comment hidden (mozreview-request)
(Assignee)

Comment 4

2 years ago
(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.
(Assignee)

Updated

2 years ago
Assignee: nobody → nchevobbe
Priority: -- → P1
Status: NEW → ASSIGNED
Whiteboard: [new-console] [triage] → [new-console]

Comment 6

2 years ago
mozreview-review
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.
Attachment #8857862 - Flags: review?(bgrinstead) → review+

Comment 7

2 years ago
Pushed by nchevobbe@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/a8a257ba1903
Delay scrollToBottom on next tick in console init. r=bgrins
https://hg.mozilla.org/mozilla-central/rev/a8a257ba1903
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
status-firefox55: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 55
Iteration: --- → 55.3 - Apr 17
Whiteboard: [new-console] → [console-html]
Flags: qe-verify? → qe-verify-

Updated

6 months ago
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.