Closed Bug 1400176 Opened 2 years ago Closed 2 years ago

JSTerm is slow when the console output has a lot of messages

Categories

(DevTools :: Console, defect, P2)

defect

Tracking

(firefox57 fixed, firefox58 fixed)

RESOLVED FIXED
Firefox 58
Tracking Status
firefox57 --- fixed
firefox58 --- fixed

People

(Reporter: nchevobbe, Assigned: bgrins)

References

Details

(Whiteboard: [reserve-console-html])

Attachments

(1 file)

Steps to reproduce:

1. Open the console
2. Enter `Array.from({length: 500}).forEach((_, i) => console.log("log message", i))` . That will log 500 messages in the console output
3. Wait until all messages are displayed
4. In the console input (aka JSTerm), enter `console`


Expected results:
The `console` word appears as I type it

Actual results:
I see some lag when typing

---

Here's a profile corresponding to the STR : https://perfht.ml/2vYb4mc

Typing `console` takes ~1s to complete, which seems a lot.

Here's another profile recorded with the STR, except I didn't entered `console` but a simple `q` key stroke : https://perfht.ml/2vXArVf

`resizeInput` takes 127ms to complete, which explains the lag we are seeing.
Priority: -- → P2
Whiteboard: [reserve-console-html]
Bug 1409413 might help here, let's profile again when it lands
Depends on: 1409413
This makes a big difference, so I'd like to get this landed and uplifted (and we can land Bug 1409413 without uplift).  You can tell by turning paint flashing on - without this change we reflow every message every time another message is added, or even when any text is entered in the input.

Note: I removed #app-wrapper from the rule since I believe it is unnecessary in there - there won't be a padding or margin anyway, and we have height: 100% in the #app-wrapper rule below
Assignee: nobody → bgrinstead
Status: NEW → ASSIGNED
Comment on attachment 8919889 [details]
Bug 1400176 - Prevent extra reflows by resetting html and body to display:block;

https://reviewboard.mozilla.org/r/190834/#review196202

This is looking good, I tried different layout and actions and I don't feel a lag anymore.
For posterity, here's a profile with the STR from Comment 0 , with th e patch applied https://perfht.ml/2yzN2C8
Attachment #8919889 - Flags: review?(nchevobbe) → review+
How can we prevent regression on this ? This looks easy to break
Pushed by bgrinstead@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/4d12126e6207
Prevent extra reflows by resetting html and body to display:block;r=nchevobbe
https://hg.mozilla.org/mozilla-central/rev/4d12126e6207
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 58
Comment on attachment 8919889 [details]
Bug 1400176 - Prevent extra reflows by resetting html and body to display:block;

Approval Request Comment
[Feature/Bug causing the regression]: Network logging in the new console UI (landed in 57)
[User impact if declined]: Slow message adding and typing in the console input
[Is this code covered by automated tests?]: No, it's CSS only
[Has the fix been verified in Nightly?]: Yes
[Needs manual test from QE? If yes, steps to reproduce]: No
[List of other uplifts needed for the feature/fix]: None
[Is the change risky?]: No
[Why is the change risky/not risky?]: It's limited to the devtools webconsole, and it's a very small CSS fix to prevent applying flexbox to the body and html elements. We won't require this change anymore once we stop loading the offending stylesheet in Bug 1409413, but that patch will not need an uplift since this one is simpler and fixes the performance issue.
[String changes made/needed]:
Attachment #8919889 - Flags: approval-mozilla-beta?
Comment on attachment 8919889 [details]
Bug 1400176 - Prevent extra reflows by resetting html and body to display:block;

Recent regression, low risk, Beta57+
Attachment #8919889 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.