1418374 - WebRender: Memory leak on GPU process

Status: RESOLVED FIXED

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

Description

[Kacper Michajłow [:kasper93]]

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

Comment 1

[Jerry Shih[:jerry] (UTC+8) (inactive)]

Could you check if this is the blob image leak?

Comment 2

[Ethan Lin[:ethlin]]

It's blob image only. I'll take a look.

Comment 3

[Ethan Lin[:ethlin]]

Attachment: Bug 1418374 - Discard blob image key when we create another new one.

Review commit: https://reviewboard.mozilla.org/r/201474/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/201474/

Comment 4

[Ethan Lin[:ethlin]]

Comment on attachment 8930343 [details]
Bug 1418374 - Discard blob image key when we create another new one.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/201474/diff/1-2/

Comment 5

[Kartikaya Gupta (email:[e-mail])]

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.

Comment 6

[Ethan Lin[:ethlin]]

(In reply to Kartikaya Gupta (email:[e-mail]) 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.

Comment 7

[Ethan Lin[:ethlin]]

Comment on attachment 8930343 [details]
Bug 1418374 - Discard blob image key when we create another new one.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/201474/diff/2-3/

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/1a65a4910d55
Discard blob image key when we create another new one. r=kats

Comment 9

[Andrei Ciure[:aciure]]

https://hg.mozilla.org/mozilla-central/rev/1a65a4910d55

Comment 10

[Kartikaya Gupta (email:[e-mail])]

Did you hand-generate the change to webrender_ffi_generated.h?

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/0b60a5360f79
Follow-up to ensure cbindgen generates the != operator for ImageKey. r=me and DONTBUILD

Comment 12

[Stefan Hindli [:stefan_hindli]]

https://hg.mozilla.org/mozilla-central/rev/0b60a5360f79