Closed Bug 1418374 Opened 4 years ago Closed 4 years ago

WebRender: Memory leak on GPU process

Categories

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

59 Branch
Unspecified
Windows 10
defect

Tracking

()

RESOLVED FIXED
mozilla59
Tracking Status
firefox57 --- unaffected
firefox59 --- fixed

People

(Reporter: kasper93, Assigned: ethlin)

References

()

Details

(Whiteboard: [wr-mvp] [gfx-noted])

Attachments

(1 file)

Hi,

There seem to be quite big memory leak after enabling WR. 

I have enabled:
gfx.webrender.enabled
gfx.webrender.blob-images


STR:

1. Go to http://browserbench.org/MotionMark/ 
2. Wait (don't start benchmark, just wait)

On my system memory usage jumps up 800MB+ every time background color on the page changes. Memory is classified as heap-unclassified under GPU process. After some time I got and it keeps going.

11,427,443,856 B (99.97%) ── heap-unclassified

Let me know if you need more information. But I believe you will be able to reproduce easily.

Best Regards,
Kacper
Severity: blocker → normal
Priority: P1 → P2
Could you check if this is the blob image leak?
Flags: needinfo?(kechen)
Whiteboard: [gfx-noted]
It's blob image only. I'll take a look.
Assignee: nobody → ethlin
Flags: needinfo?(kechen)
Blocks: 1414890
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Priority: P2 → P1
Whiteboard: [gfx-noted] → [wr-mvp] [gfx-noted]
Comment on attachment 8930343 [details]
Bug 1418374 - Discard blob image key when we create another new one.

https://reviewboard.mozilla.org/r/201474/#review206874

::: gfx/layers/wr/WebRenderUserData.cpp:163
(Diff revision 2)
>  }
>  
> +void
> +WebRenderImageData::SetKey(const wr::ImageKey& aKey)
> +{
> +  if (mKey) {

If aKey is always going to be different from mKey, it would be good have a MOZ_ASSERT to that effect. Otherwise it would be good to have an early-exit if mKey == aKey. Either way we should make sure this code doesn't run if mKey == aKey because then we'll end up using a discarded key which will probably result in hard-to-debug failures.
Attachment #8930343 - Flags: review?(bugmail) → review+
(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #5)
> Comment on attachment 8930343 [details]
> Bug 1418374 - Discard blob image key when we create another new one.
> 
> https://reviewboard.mozilla.org/r/201474/#review206874
> 
> ::: gfx/layers/wr/WebRenderUserData.cpp:163
> (Diff revision 2)
> >  }
> >  
> > +void
> > +WebRenderImageData::SetKey(const wr::ImageKey& aKey)
> > +{
> > +  if (mKey) {
> 
> If aKey is always going to be different from mKey, it would be good have a
> MOZ_ASSERT to that effect. Otherwise it would be good to have an early-exit
> if mKey == aKey. Either way we should make sure this code doesn't run if
> mKey == aKey because then we'll end up using a discarded key which will
> probably result in hard-to-debug failures.

I will add MOZ_ASSERT.
Pushed by ethlin@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/1a65a4910d55
Discard blob image key when we create another new one. r=kats
https://hg.mozilla.org/mozilla-central/rev/1a65a4910d55
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla59
Did you hand-generate the change to webrender_ffi_generated.h?
Pushed by kgupta@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/0b60a5360f79
Follow-up to ensure cbindgen generates the != operator for ImageKey. r=me and DONTBUILD
You need to log in before you can comment on or make changes to this bug.