Preloaded new tab page thinks it's visible and redraws when the window is resized
Categories
(Firefox :: New Tab Page, defect, P3)
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:
- Create a new profile.
- Navigate to https://profiler.firefox.com/ , close all other open tabs.
- (Optional: Install and start the Gecko Profiler add-on.)
- Resize the browser window
- (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.
Comment 1•6 years ago
|
||
The priority flag is not set for this bug.
:tspurway, could you have a look please?
For more information, please visit auto_nag documentation.
Updated•6 years ago
|
Updated•6 years ago
|
Comment 2•6 years ago
|
||
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.
| Reporter | ||
Comment 3•6 years ago
|
||
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.
Comment 4•6 years ago
|
||
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)
Comment 5•6 years ago
|
||
The priority flag is not set for this bug.
:tspurway, could you have a look please?
For more information, please visit auto_nag documentation.
Updated•6 years ago
|
Updated•6 years ago
|
Updated•6 years ago
|
Updated•4 years ago
|
Updated•3 years ago
|
Updated•3 years ago
|
Updated•2 years ago
|
Updated•2 years ago
|
| Assignee | ||
Comment 7•1 year ago
|
||
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?
Comment 8•1 year ago
|
||
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
| Assignee | ||
Comment 10•1 year ago
|
||
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.
Updated•1 year ago
|
| Assignee | ||
Comment 11•1 year ago
|
||
So... There was some code unconditionally setting renderLayers=true, which is what's causing this.
| Assignee | ||
Updated•1 year ago
|
Comment 12•1 year ago
|
||
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.
| Assignee | ||
Updated•1 year ago
|
Comment 13•1 year ago
|
||
From Bug 1952787 comment 11, PreloadedBrowser always requests vsync enabled. It is not good.
Comment 14•11 months ago
|
||
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?
| Assignee | ||
Comment 15•11 months ago
|
||
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
renderLayersconditional 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? :)
Comment 16•11 months ago
|
||
(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).
Updated•14 days ago
|
Description
•