Closed Bug 1539366 Opened 7 months ago Closed 5 months ago

Typing in Phabricator textarea on long pages is super slow

Categories

(Core :: Web Painting, defect, P2)

defect

Tracking

()

RESOLVED FIXED
mozilla69
Tracking Status
firefox68 --- fixed
firefox69 --- fixed

People

(Reporter: nchevobbe, Assigned: mattwoodrow)

Details

Attachments

(3 files)

Steps to reproduce

  1. Go to https://phabricator.services.mozilla.com/D24930
  2. Scroll all the way to the bottom to access the text box
  3. Focus the input and type a letter

Results

It takes more than 10s for the typed letter to appear in the text box.
Here's a profile of typing a single letter in there: http://bit.ly/2HTsn0p


Filing on Graphics as it looks like this is the culprit here.

Seems to be more of a DisplayList-building issue around painting backgrounds, with a huge amount of time going into nsBlockInFlowLineIterator::nsBlockInFlowLineIterator. Moving to Web Painting.

(FWIW, just the initial rendering and scrolling to the bottom of that page takes "forever" (almost) here, this isn't specifically about typing.)

Component: Graphics → Web Painting
Assignee: nobody → matt.woodrow
Priority: -- → P2

Without this the restore doesn't have any effect as the next frame sees that coord has mIsValid=false

Pushed by mwoodrow@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/e8b188fd600d
Use SetCoord to restore mPIStartBorderData's coord so that it also set mValid. r=mats
https://hg.mozilla.org/integration/autoland/rev/d32769ee5991
Use a temporary object to compute the mPIStartBorderData coords so that we don't clobber the regular state. r=mats
https://hg.mozilla.org/integration/autoland/rev/9d54e34f7f66
Avoid calling AreOnSameLine once we know we've already changed lines since it can be expensive to compute. r=mats
Status: NEW → RESOLVED
Closed: 5 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla69

Is this worth an uplift to beta (and safe enough)?

Flags: needinfo?(matt.woodrow)

Comment on attachment 9056790 [details]
Bug 1539366 - Avoid calling AreOnSameLine once we know we've already changed lines since it can be expensive to compute. r?mats

Beta/Release Uplift Approval Request

  • User impact if declined: Slow display list building on some inline heavy pages, include Phabricator.
  • Is this code covered by automated tests?: Yes
  • Has the fix been verified in Nightly?: No
  • Needs manual test from QE?: No
  • If yes, steps to reproduce:
  • List of other uplifts needed: None
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): Fairly simple performance optimization, just fixes cases where the existing cache wasn't being hit.
  • String changes made/needed: None
Flags: needinfo?(matt.woodrow)
Attachment #9056790 - Flags: approval-mozilla-beta?
Attachment #9056788 - Flags: approval-mozilla-beta?
Attachment #9056789 - Flags: approval-mozilla-beta?

Comment on attachment 9056788 [details]
Bug 1539366 - Use SetCoord to restore mPIStartBorderData's coord so that it also set mValid. r?mats

painting perf fixes, approved for 68.0b8

Attachment #9056788 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Attachment #9056789 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Attachment #9056790 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
You need to log in before you can comment on or make changes to this bug.