Closed
Bug 1400176
Opened 7 years ago
Closed 7 years ago
JSTerm is slow when the console output has a lot of messages
Categories
(DevTools :: Console, defect, P2)
DevTools
Console
Tracking
(firefox57 fixed, firefox58 fixed)
RESOLVED
FIXED
Firefox 58
People
(Reporter: nchevobbe, Assigned: bgrins)
References
Details
(Whiteboard: [reserve-console-html])
Attachments
(1 file)
(deleted),
text/x-review-board-request
|
nchevobbe
:
review+
ritu
:
approval-mozilla-beta+
|
Details |
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.
Reporter | ||
Updated•7 years ago
|
Priority: -- → P2
Whiteboard: [reserve-console-html]
Reporter | ||
Comment 1•7 years ago
|
||
Bug 1409413 might help here, let's profile again when it lands
Depends on: 1409413
Comment hidden (mozreview-request) |
Assignee | ||
Comment 3•7 years ago
|
||
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
Reporter | ||
Comment 4•7 years ago
|
||
mozreview-review |
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+
Reporter | ||
Comment 5•7 years ago
|
||
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
Comment 7•7 years ago
|
||
bugherder |
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
status-firefox58:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 58
Assignee | ||
Comment 8•7 years ago
|
||
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+
Comment 10•7 years ago
|
||
bugherder uplift |
Updated•6 years ago
|
Product: Firefox → DevTools
You need to log in
before you can comment on or make changes to this bug.
Description
•