Closed Bug 1627739 Opened 4 months ago Closed 4 months ago

With WebRender enabled, off-screen GIFs cause memory leak

Categories

(Core :: Graphics: WebRender, defect, P2)

74 Branch
defect

Tracking

()

VERIFIED FIXED
mozilla77
Tracking Status
firefox-esr68 --- unaffected
firefox75 --- wontfix
firefox76 --- verified
firefox77 --- verified

People

(Reporter: nyanpasu64, Assigned: aosmond)

References

(Blocks 1 open bug, Regression)

Details

(Keywords: regression)

Attachments

(2 files)

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:74.0) Gecko/20100101 Firefox/74.0

Steps to reproduce:

Actual results:

  • Looking at Task Manager, the tab's content process memory usage rises rapidly (100-200 MB/s).
  • Every time you interact with the page (focus/unfocus the window, scroll the page, move your mouse), the memory usage resets to normal.
  • If the GIF is visible, memory does not leak.
  • If you do nothing and let memory leak, Firefox's memory usage can go into the gigabytes. I've seen 3-10GB, to the point that other applications were failing to allocate memory.

In Task Manager, working set rises, but active private working set does not.

I took a memory dump of the content process when 3 gigabytes was allocated. By loading the memory dump into GIMP as "Raw image data", I saw that much of the memory was taken up by RGBA frames of the GIF (2732 pixels wide)

Expected results:

Memory usage doesn't rise without limit.

Bugbug thinks this bug should belong to this component, but please revert this change in case of error.

Component: Untriaged → Graphics: WebRender
Product: Firefox → Core
Blocks: wr-perf
Priority: -- → P3
Status: UNCONFIRMED → NEW
Ever confirmed: true
Flags: needinfo?(aosmond)
Regressed by: 1599656

Seems to be a bug in recycling. If I turn off image.animated.decode-on-demand.recycle, the memory consumption is stable (well it is always freeing / reallocating buffers but doesn't increase above a reasonable ceiling).

Assignee: nobody → aosmond
Flags: needinfo?(aosmond)
Priority: P3 → P2
Status: NEW → ASSIGNED

Ultimately what appears to happen is:

  1. FrameAnimator::GetCompositedFrame is called, we advance the frame
  2. Eventually RenderRootStateManager::FlushAsyncResourceUpdates is called to flush the changes
  3. In WebRenderBridgeParent::RecvUpdateResources we now will wait for a Checkpoint::FrameTexturesUpdated notification to release the old buffer for recycling
  4. The notification never comes, so when we need to advance the animation again, we eventually run out of buffers and allocate a new one; this continues forever

The notification never comes because it decided the frame did not need to be updated. I think when we issue a layer transaction we hit this path:

https://searchfox.org/mozilla-central/rev/2cd2d511c0d94a34fb7fa3b746f54170ee759e35/gfx/wr/webrender/src/render_backend.rs#1677

which causes us not to re-render the next frame. As a result, we never actually flush the pending texture updates (themselves which appear to be empty), and we are always defering the future notifications.

The FrameTexturesUpdated event is intended to be issued when any
necessary texture cache updates have been completed for the current
frame. Originally we would simply check if the pending_texture_updates
vector in Renderer is empty. However the updates themselves could be
nops, and not trigger a timely frame render to occur, causing the
notifications to be delayed indefinitely. Now we track if there are
actually any pending specifically texture cache updates and only defer
the notification if true.

Pushed by aosmond@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/3e0fdd643f2e
Do not delay FrameTexturesUpdated notifications for empty updates. r=kvark
Status: ASSIGNED → RESOLVED
Closed: 4 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla77

The patch landed in nightly and beta is affected.
:aosmond, 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?(aosmond)

Comment on attachment 9140275 [details]
Bug 1627739 - Do not delay FrameTexturesUpdated notifications for empty updates.

Beta/Release Uplift Approval Request

  • User impact if declined: May see memory increase when animated images are in the visible tab/window, but are in the current viewport when using WebRender.
  • Is this code covered by automated tests?: Yes
  • Has the fix been verified in Nightly?: Yes
  • Needs manual test from QE?: No
  • If yes, steps to reproduce:
  • List of other uplifts needed: None
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): It is a minor tweak to how we issue WebRender notifications, we send them a little sooner than we did before, and only when the state could not possibly have changed.
  • String changes made/needed:
Flags: needinfo?(aosmond)
Attachment #9140275 - Flags: approval-mozilla-beta?

Comment on attachment 9140275 [details]
Bug 1627739 - Do not delay FrameTexturesUpdated notifications for empty updates.

Fixes a possible memory leak in WebRender. Approved for 76.0b7.

Attachment #9140275 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Flags: qe-verify+
QA Whiteboard: [qa-triaged]

I can't seem to be able to reproduce on my system. I have attempted the steps in comment 0 in Firefox Release v74.0 and Firefox Release v75.0. Checked both the about:performance tab and the Windows Task Manager and there was no rapid increase of used memory.

Does this issue still reproduce for you? Can you reproduce in the latest Release Version of Firefox, v75.0?
If yes, could you attempt to reproduce is in the latest Beta and Nightly versions. Hopefully, it is now fixed in those versions.
Thank you for your contribution!

Flags: needinfo?(nyanpasu64)

Yes I still get the issue in Firefox 75.0 on Windows. I killed Firefox after 1.6GB of allocations.

The bug also occurs on a Linux machine with 8GB of RAM. The memory leak (ksysguard Shared Mem, not Memory) sometimes resolves after 1GB, and sometimes continues to 4GB at which point the tab crashes (from OOM I assume).

The issue does not occur on Firefox Developer 76.0b7 (64-bit) on Windows. I did not test Developer or Nightly on Linux.

Flags: needinfo?(nyanpasu64)

Verifying firefox76 based on the previous comment. Thank you very much, reporter.
It would be awesome if you could also confirm that it doesn't occur in the latest Nightly, eighter, so we can close this issue properly:
http://archive.mozilla.org/pub/firefox/nightly/latest-mozilla-central/

Flags: needinfo?(nyanpasu64)

I can't seem to be able to reproduce on my system.

Don't know why. Opening Stack Overflow in a private window, the dark mode banner is initially animated, but the memory leak eventually happens anyway.

Anyway no leak in Firefox Nightly 77.0a1 (2020-04-22) (64-bit), on Windows 10 x64. Seems fixed there too.

Flags: needinfo?(nyanpasu64)

Much appreciated!

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