Open Bug 1548683 Opened 6 years ago Updated 14 days ago

Preloaded new tab page thinks it's visible and redraws when the window is resized

Categories

(Firefox :: New Tab Page, defect, P3)

defect

Tracking

()

Tracking Status
firefox68 --- affected

People

(Reporter: mstange, Assigned: emilio, NeedInfo)

References

(Blocks 4 open bugs)

Details

(Keywords: perf, perf:resource-use, power, Whiteboard: [hnt])

Attachments

(1 file)

Profile: https://perfht.ml/2ZOC6eV

Steps to reproduce:

  1. Create a new profile.
  2. Navigate to https://profiler.firefox.com/ , close all other open tabs.
  3. (Optional: Install and start the Gecko Profiler add-on.)
  4. Resize the browser window
  5. (Optional: Grab a profile.)

Expected results:
Only the browser window document and the foreground tab should be painted during the resize.

Actual results:
The profile shows painting in the Privileged Content process, repainting about:newtab. about:newtab is not visible.

The priority flag is not set for this bug.
:tspurway, could you have a look please?

For more information, please visit auto_nag documentation.

Flags: needinfo?(tspurway)
Component: New Tab Page → Activity Streams: Newtab
Flags: needinfo?(tspurway)

This is kind of intended. The new tab page that is pre-rendered will also be resized in the background. If there are perf issues, however, we could debounce these resize operations to happen after the user is done resizing.

Flags: needinfo?(mstange)

I see. So there are two parts that happen during resizing: 1. Reflow at the new size, and 2. Rendering / rasterization.
Doing 1 is fine. I'm more concerned about 2. Having the rasterized content around at all times also takes up quite a bit of memory, at least with non-WebRender.

If about:newtab were not preloaded at all, opening a new tab would take up time in the following stages: 1. Pageload, 2. JavaScript execution / DOM "render", 3. Layout, 4. Paint. Preloading it in an "inactive" tab would prepare 1-3, but leave 4 to happen at the time that the tab is actually opened. Preloading it in an "active" tab (which seems to be what's happening now) prepares all 1-4; so it saves the painting time but pays the painting memory cost.

Do you know which bug made the change to have it pre-rendered? I'm curious about the performance considerations that went into it.

Flags: needinfo?(mstange)

Looks like bug 791670 is where most of the work to have the page preloaded by default. The main desire is to make the page appear immediately ready when the user opens a new tab.

I wonder if something simple like setting the page to opacity: 0 could avoid paint without affecting layout (so not display: none)

See Also: → 791670

The priority flag is not set for this bug.
:tspurway, could you have a look please?

For more information, please visit auto_nag documentation.

Flags: needinfo?(tspurway)
Flags: needinfo?(tspurway)
Priority: -- → P3
Component: Activity Streams: Newtab → New Tab Page
No longer blocks: pocket-newtab
See Also: → 1696942
See Also: 1696942
Severity: normal → --
Severity: -- → S4
Whiteboard: [hnt]

We should just mark it as an inactive tab. That would automatically:

  • Throttle its refresh driver.
  • Avoid rendering it unnecessarily.
  • After bug 1917458, avoid resizing it eagerly.

Mike, I see you're somewhat familiar with this code, do you know where the <browser> or browsingContext holding this page lives?

Flags: needinfo?(mconley)

Hi! Yes, I remember this stuff.

The <browser> is part of the normal tabpanels list, but isn't associated with a pre-existing <tab> element: https://searchfox.org/mozilla-central/rev/a13db27562f9237db97e2ea5b01dc879d5b55b74/browser/components/tabbrowser/NewTabPagePreloading.sys.mjs#63-70

Flags: needinfo?(mconley)

I'll try to take a look

Flags: needinfo?(emilio)

It wastes more resources that it wins. If we knew we were about to
display the new tab page, then it'd be worth doing, but lacking that...

Do some minor clean-up while at it.

Assignee: nobody → emilio
Status: NEW → ASSIGNED

So... There was some code unconditionally setting renderLayers=true, which is what's causing this.

Assignee: emilio → nobody
Status: ASSIGNED → NEW
Flags: needinfo?(emilio)
See Also: → 1447797, 1580117
Assignee: nobody → emilio

The following patch is waiting for review from an inactive reviewer:

ID Title Author Reviewer Status
D222757 Bug 1548683 - Don't set renderLayers=true on preloaded newtabs. r=mconley! emilio mconley: Back Jan 4, 2025

:emilio, could you please find another reviewer?

For more information, please visit BugBot documentation.

Flags: needinfo?(emilio)
Flags: needinfo?(emilio)

From Bug 1952787 comment 11, PreloadedBrowser always requests vsync enabled. It is not good.

Blocks: 1952787

emilio,

I've gotten our New Tab PM's up-to-speed on this. Their question is: what do we gain by removing this perceived performance optimization? New Tab has a clear sense of what we lose (we lose the smoothness of opening a newtab on top of an existing newtab), but we don't have a clear sense of what we gain by this change. We've identified this as a potentially wasteful expenditure of CPU / GPU resources. Do we have a sense of how wasteful?

Flags: needinfo?(emilio)

Do we have a sense of how wasteful?

There are two issues:

  • The new tab page is active even though it's not visible. This is not the worst (and it's what gets you the perceived performance win), but it has shown up in profiles. The new tab page has some animations (some even built-in, for things like scrollbars) that keep firefox ticking if they fire.

  • The preloaded new tab page seems to remain active even if the Firefox window is occluded or minimized (see bug 1952787). Arguably that's fixable: we'd need to make renderLayers conditional on whether the window is minimized dynamically (rather than just at the point the newtab page loads here).

(2) is the one that showed up lately, because it keeps HW vsync running even when hidden / minimized / occluded. That is as I said fixable, but it wouldn't make this less of a one-off, and it would still be rather prone to breaking. Even with it fixed, I think it's rather risky...

(1) has shown up in the past (specially if you look at profiles and see "why are we painting the new tab page at all")? I do think keeping this page awake all the time is not worth the cost. If only, because it moves the "don't cause terrible battery usage" responsibility to the new tab page. Any periodic content update or animations that the new tab page does can tank the battery usage, and we're unlikely to notice because a lot of tests including talos disable this.

Hopefully that clarifies my view on this? :)

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

(In reply to Mike Conley (:mconley) (:⚙️) from comment #14)

We've identified this as a potentially wasteful expenditure of CPU / GPU resources. Do we have a sense of how wasteful?

From memory (I haven't profiled today to verify if it's still accurate), I think the preloaded new tab gets repainted while resizing the browser window, and also whenever the system theme changes between dark and light (happens at sunset on Mac. Also seems to happen when (un)locking the session on Windows).

See Also: → 1975272
See Also: → 1991327
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: