Closed Bug 1646222 Opened 7 months ago Closed 3 months ago

Google Sheets hangs for 3-10 seconds after maximizing or unmaximizing browser window

Categories

(Core :: Graphics: WebRender, defect)

Unspecified
Windows 10
defect

Tracking

()

VERIFIED FIXED
84 Branch
Tracking Status
firefox79 --- wontfix
firefox81 --- wontfix
firefox82 --- wontfix
firefox83 --- wontfix
firefox84 --- verified

People

(Reporter: cpeterson, Assigned: emilio)

References

(Blocks 1 open bug, )

Details

(Whiteboard: [qf:p2:responsiveness])

Attachments

(1 file)

Basic information

Steps to Reproduce:

  1. Open a Google Sheet, such as https://docs.google.com/spreadsheets/d/1w5DRoWOkctcsVak0c_uzzmU2s6ofxxRZuUZMDuIML_w/edit#gid=642635812.
  2. Maximize and unmaximize ("Restore Down" on Windows) the browser window.
  3. After the window is resized, try clicking in spreadsheet cells.

Expected Results:

The spreadsheet should resize and be clickable in less than one second (like in Chrome).

Actual Results:

The spreadsheet hangs for 3-10 seconds and then quickly processes the pending mouse clicks. Resizing the window by dragging the window edge does not seem to cause hangs as long as maximizing and unmaximizing does.

I'm testing 79 Nightly on Windows 10. The hangs did not seem as long in the Firefox 77 Release builds, so I tried bisecting for a perf regression using mozregression. Unfortunately, the long hangs seem to happen in 77 Nightly also, so perhaps some Nightly-only diagnostic checks are causing the hangs?


More information

Profile URL: https://share.firefox.dev/3fwzaL7

Basic systems configuration:

OS version: Windows 10.0.19041 Build 19041

GPU model: Intel(R) UHD Graphics 630

Number of cores: Intel(R) Core(TM) i9-8950HK CPU @ 2.90GHz, 2904 Mhz, 6 Core(s), 12 Logical Processor(s)

Amount of memory (RAM):
Installed Physical Memory (RAM) 32.0 GB
Total Physical Memory 31.7 GB
Available Physical Memory 13.5 GB
Total Virtual Memory 36.6 GB
Available Virtual Memory 7.54 GB

How many Google sheet tabs do you have open?

The time is spent inside SVG-as-image drawing. We're drawing the same SVG images over and over, to account for "media feature changes" that probably don't even affect the SVG contents in any way.

Component: Performance → Layout: Images, Video, and HTML Frames
Flags: needinfo?(cpeterson)
Whiteboard: [qf] → [qf:p2:responsiveness]

When the invalidation happens, I made it call UpdateImageContainer which repaints the SVG using the cached parameters, as we require a display list rebuild to change those. Do we have a sense of what MediaFeatureChangeReason values could cause a repaint with the same input parameters (SVGContext, size, draw flags) and get a different result, without an accompanying display list rebuild? Without the rebuild, the scene will keep using the image container with the wrong cached result. If we know a rebuild is coming, or the result isn't different, I can just remove the UpdateImageContainer entirely....

So... regressed by bug 1598480?

(In reply to Andrew Osmond [:aosmond] from comment #2)

Without the rebuild, the scene will keep using the image container with the wrong cached result. If we know a rebuild is coming, or the result isn't different, I can just remove the UpdateImageContainer entirely....

Can we just trigger a display list rebuild when this notification comes in, e.g. by calling SchedulePaint on the root frame? Then we'd pick up changes to the SVG images during that display list build and we wouldn't need to traverse images separately.

S3, I presume maximizing/unmaximizing is not so commonly used, but maybe I am wrong.

Severity: -- → S3

(In reply to Markus Stange [:mstange] from comment #1)

How many Google sheet tabs do you have open?

I can reproduce the slow resize problem with one Google sheet tab and no other tabs open.

Flags: needinfo?(cpeterson)

This might also be fixed by bug 1668875. Does it still happen?

Flags: needinfo?(cpeterson)

This does not reproduce without WR, so moving.

Blocks: wr-perf
Component: Layout: Images, Video, and HTML Frames → Graphics: WebRender

Profile from today's Nightly, with WebRender: https://share.firefox.dev/2INOHuN

Still happening. And it's not the ThemeChanged notification. It goes nsPresContext::SizeModeChanged -> nsPresContext::MediaFeatureValuesChangedAllDocuments -> mozilla::dom::ImageTracker::MediaFeatureValuesChangedAllDocuments.

Emilio, would you like to take a look?

Flags: needinfo?(cpeterson) → needinfo?(emilio)

(In reply to Markus Stange [:mstange] from comment #7)

This might also be fixed by bug 1668875. Does it still happen?

(In reply to Markus Stange [:mstange] from comment #9)

Still happening. And it's not the ThemeChanged notification. It goes nsPresContext::SizeModeChanged -> nsPresContext::MediaFeatureValuesChangedAllDocuments -> mozilla::dom::ImageTracker::MediaFeatureValuesChangedAllDocuments.

FWIW, I originally filed this bug and I can no longer reproduce the hang in 84 Nightly, but I can still reproduce in 83 Beta and 82 Release.

See Also: → 1668875

This should've been fixed by bug 1668875, did that profile have that commit?

Flags: needinfo?(emilio) → needinfo?(mstange.moz)

Anyhow even with that we could do better, patch incoming.

sizemode/displaymode media queries only affect a given browsing context
tree so there's no need to propagate the change to images in that case.

Assignee: nobody → emilio
Status: NEW → ASSIGNED
Pushed by ealvarez@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/7b3e9881ab73
Don't invalidate vector images as a result of size/display mode changes. r=tnikkel
Status: ASSIGNED → RESOLVED
Closed: 3 months ago
Resolution: --- → FIXED
Target Milestone: --- → 84 Branch

(In reply to Emilio Cobos Álvarez (:emilio) from comment #11)

This should've been fixed by bug 1668875, did that profile have that commit?

Oh, it seems the profile was from a Firefox Release 82 build, not from a Nightly.

Anyway, thanks for fixing this even harder!

Flags: needinfo?(mstange.moz)

The patch landed in nightly and beta is affected.
:emilio, is this bug important enough to require an uplift?
If not please set status_beta to wontfix.

For more information, please visit auto_nag documentation.

Flags: needinfo?(emilio)

I believe we should decide whether to uplift bug 1668875 instead.

Flags: needinfo?(emilio)

(In reply to Emilio Cobos Álvarez (:emilio) from comment #18)

I believe we should decide whether to uplift bug 1668875 instead.

Note that we're about to build the final 83 beta of the cycle, so we need to make a decision there Really Soon Now.

(In reply to Ryan VanderMeulen [:RyanVM] from comment #19)

(In reply to Emilio Cobos Álvarez (:emilio) from comment #18)

I believe we should decide whether to uplift bug 1668875 instead.

Note that we're about to build the final 83 beta of the cycle, so we need to make a decision there Really Soon Now.

There's probably no rush to uplift if the code change is big or risky. 84 will ship in just five weeks.

This Google Sheets perf issue is longstanding issue and users are probably not resizing their Firefox windows that often.

Reproduced the issue using Firefox 79.0a1 (20200616215006) on Windows 10x64 and STR from comment 0.
The issue is verified fixed with Firefox 84.0b3 (20201119195818) on Windows 10x64, macOS 10.12, and Ubuntu 18.04. There is no hang after maximizing/ restoring the window and clicking inside google sheet cells.

Status: RESOLVED → VERIFIED
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.