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

Status: VERIFIED FIXED

(Core :: Graphics: WebRender, defect)

Description

[Chris Peterson [:cpeterson]]

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

Comment 1

[Markus Stange [:mstange]]

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.

Comment 2

[Andrew Osmond [:aosmond] (he/him)]

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....

Comment 3

[Emilio Cobos Álvarez (:emilio)]

So... regressed by bug 1598480?

Comment 4

[Markus Stange [:mstange]]

(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.

Comment 5

[Hiroyuki Ikezoe (:hiro)]

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

Comment 6

[Chris Peterson [:cpeterson]]

(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.

Comment 7

[Markus Stange [:mstange]]

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

Comment 8

[Bas Schouten (:bas.schouten)]

This does not reproduce without WR, so moving.

Comment 9

[Markus Stange [:mstange]]

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?

Comment 10

[Chris Peterson [:cpeterson]]

(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.

Comment 11

[Emilio Cobos Álvarez (:emilio)]

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

Comment 12

[Emilio Cobos Álvarez (:emilio)]

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

Comment 13

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1646222 - Don't invalidate vector images as a result of size/display mode changes. r=#style,tnikkel

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.

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/7b3e9881ab73
Don't invalidate vector images as a result of size/display mode changes. r=tnikkel

Comment 15

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/7b3e9881ab73

Comment 16

[Markus Stange [:mstange]]

(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!

Comment 17

[BugBot [:suhaib / :marco/ :calixte]]

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.

Comment 18

[Emilio Cobos Álvarez (:emilio)]

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

Comment 19

[Ryan VanderMeulen [:RyanVM]]

(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.

Comment 20

[Chris Peterson [:cpeterson]]

(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.

Comment 21

[Alexandru Trif, Desktop QA [:atrif]]

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.