1924841 - Too much CPU time spent compressing thumbnails

Status: RESOLVED FIXED

(Firefox for Android :: Tabs, defect)

Description

[Markus Stange [:mstange]]

Profile: https://share.firefox.dev/4873l86

Fenix seems to store tab thumbnails to disk very frequently. Every time it does this, it spends CPU time to WebP-encode the thumbnails.

If you profile Firefox using simpleperf, it shows up very clearly on the ThumbnailStorag threads. I see it right after startup, and every time I go to the tabs tray.

My guess is that we're storing all thumbnails instead of just thumbnails that may have changed. I have 50 tabs open in my tabs tray, but only one tab is actually loaded. The other tabs are not loaded and couldn't have changed their thumbnail, so they don't need to be stored.

Comment 1

[Markus Stange [:mstange]]

Hmm, in a profile with just four tabs I see 350ms spent here. Maybe we are just doing it for one image, but maybe the image is really big or the encoding is really slow. https://share.firefox.dev/4eJrmEQ

Comment 2

[BugBot [:suhaib / :marco]]

The severity field is not set for this bug.
:007, could you have a look please?

For more information, please visit BugBot documentation.

Comment 3

[Noah Bond [:007]]

Tagging a bug that might be adjacently related

Comment 4

[Markus Stange [:mstange]]

The next step here would be to add a profiler marker covering the compression of a single image, with the pixel size of the image included in the marker details. This will let us answer all the questions from comment 1.

Comment 5

[Kaya [:kaya]]

I observe that we are saving the thumbnail of the most recently used (visible) tab whenever we open the tabs tray. That operation does not try to save the thumbnails of all of the tabs. I think this is done in order to be able to update the thumbnail in case the visible tab's state change (e.g scroll state) so that it will show the most recent state of the page at the time of next tabs tray launch. One thing we could possibly do is if no state change happened on the visible tab between the tabs tray openings, we could bypass the saving op. I'll also take a look at what we are trying to save and whether we can optimize the image itself to reduce the CPU, rendering time etc.

Comment 6

[Jeff Muizelaar [:jrmuizel]]

Kaya, did you get a profile with markers that you can share?

Comment 7

[Kaya [:kaya]]

could not add a marker as the op is happening in another thread (see Bug 1960730). I will come up with a temp workaround to get a profile with the markers. Worst case: I can log runtime (in nanosecs) on that thread for that operation along with the width/height of the bitmap, and we can check it out over the logcat. Keeping the ni, until I resolve this.

Comment 8

[Jeff Boek [:boek]]

Doing some timestamp logging I could see the call to compress the bitmap to WebP taking over 250ms on my Pixel Fold. It looks like others have ran into this issue as well:

https://issuetracker.google.com/issues/175943360
https://issuetracker.google.com/issues/120109351

Switching from WebP to JPG I saw similar improvements.

As for when we call this, it happens any time we a page is loaded and we have a first paint. For cold starts loading existing pages we should be able to avoid the thumbnail call, but we don't have any mechanisms in place to track this yet.

Comment 9

[Jeff Boek [:boek]]

Attachment: Bug 1924841 - Changes ThumbnailDiskCache to use JPG over WebP

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/19aa814afb4c
https://hg.mozilla.org/integration/autoland/rev/e424d3f5700b
Part 1: Changes ThumbnailDiskCache to use JPG over WebP r=android-reviewers,sfamisa

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/5e8372c6834f
https://hg.mozilla.org/integration/autoland/rev/de80f604fb08
Revert "Bug 1924841 - Part 1: Changes ThumbnailDiskCache to use JPG over WebP r=android-reviewers,sfamisa" for causing Android Components opt build bustages.

Comment 12

[Serban Stanca [:SerbanS]]

Reverted this because it was causing Android Components opt build bustages.

• Revert link
• Push with failures
• Failure Log
• Failure line: TEST-UNEXPECTED-FAIL | mozilla.components.browser.thumbnails.utils.ThumbnailDiskCacheTest.Writing and reading bitmap bytes for sdk higher than 29 | java.lang.AssertionError: expected:<WEBP_LOSSY> but was:<JPEG>

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/78f15855e508
https://hg.mozilla.org/integration/autoland/rev/b3c3369ccffc
Changes ThumbnailDiskCache to use JPG over WebP r=android-reviewers,sfamisa

Comment 14

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/b3c3369ccffc

Comment 15

[Florin Bilt]

(In reply to Cosmin Sabou [:CosminS] from comment #14)

https://hg.mozilla.org/mozilla-central/rev/b3c3369ccffc

Perfherder has detected a mozperftest performance change from push b3c3369ccffc8266b39450bbda63e16c62f01d2f.

If you have any questions, please reach out to a performance sheriff. Alternatively, you can find help on Slack by joining #perf-help, and on Matrix you can find help by joining #perftest.

Improvements:

Ratio	Test	Platform	Options	Absolute values (old vs new)
7%	shopify-applink-startup org.mozilla.fenix-cpu-time	android-hw-a55-14-0-aarch64-shippable		4,790.33 -> 4,431.33
7%	shopify-applink-startup org.mozilla.fenix-cpu-time	android-hw-a55-14-0-aarch64-shippable		4,740.25 -> 4,426.17
6%	newssite-applink-startup org.mozilla.fenix-cpu-time	android-hw-a55-14-0-aarch64-shippable		4,483.42 -> 4,227.67

Details of the alert can be found in the alert summary, including links to graphs and comparisons for each of the affected tests.

If you need the profiling jobs you can trigger them yourself from treeherder job view or ask a performance sheriff to do that for you.

You can run all of these tests on try with ./mach try perf --alert 46170

The following documentation link provides more information about this command.