1464625 - Large images retained in memory even though they are downscaled

Status: NEW

(Core :: Graphics: ImageLib, defect, P3)

Description

[ouellet.rem123]

User Agent: Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:61.0) Gecko/20100101 Firefox/61.0
Build ID: 20180524181234

Steps to reproduce:

For example, go to this website : https://mobilesyrup.com/


Actual results:

Memory usage increase to more than 1gb


Expected results:

A normal memory usage. This issue doesn't happen in Google Chrome.

Comment 1

[Damira Akh]

@mconley
I have verified this issue on Beta, version 61.0b9, build1

Gecko profile captured: https://perfht.ml/2J3bb6g 

I followed the following steps.

Steps to reproduce:
Go to website : https://mobilesyrup.com/

Actual results:
Memory usage increase to more than 1gb


What component should it go?

Comment 2

[Mike Conley (:mconley) (:⚙️)]

(In reply to Damira Akh from comment #1)
> 
> What component should it go?

Hard to say. The Gecko Profiler is not really the best tool to help us diagnose memory problems. Instead, can you reproduce the issue and produce and post a memory report from about:memory? That'll help us narrow into where the memory is going.

Comment 3

[Damira Akh]

Attachment: memory-report.json 1464625

Comment 4

[Damira Akh]

@mconley

I am attaching memory report from about:memory
I hope it will give you more info.

Comment 5

[ouellet.rem123]

Please note that this trouble happens since the first version of firefox quantum. It's not specific to a beta version of firefox 61.

Comment 6

[Mike Conley (:mconley) (:⚙️)]

It looks like we're holding a bunch of memory (about half a gig?) on decoded images? Gonna put this in imagelib for now, I guess.

Anything actionable here, aosmond?

Comment 7

[Timothy Nikkel (:tnikkel)]

Okay, this is silly. The images for the stories are huge even though they are sized down by the website. We should handle that fine with downscale during decode except we request a decode at the intrinsic size of the images, and that decode gets the CANNOT_SUBSTITUTE flag. We don't want either of those things to happen. The reason that happens is that in RasterImage::NotifyDecodeComplete when we finish a metadata decode if a full decode has been requested we issue a decode request (because we couldn't until the metadata decode was done). This decode request is at the intrinsic size and passes no flags. Which triggers this code

https://dxr.mozilla.org/mozilla-central/rev/763f30c3421233a45ef9e67a695c5c241a2c8a3a/image/RasterImage.cpp#1248

because we didn't pass the high quality scaling flag to label at as cannot substitute. And we don't get rid of these surfaces nearly as easily. We should check if there are any other decode requests that fall into this trap.

Comment 8

[Andrew Osmond [:aosmond] (he/him)]

There are a few places where we request decoding at the intrinsic size:

https://searchfox.org/mozilla-central/rev/38bcf897f1fa19c1eba441a611cf309482e0d6e5/layout/style/ImageLoader.cpp#134,139
https://searchfox.org/mozilla-central/rev/83a923ef7a3b95a516f240a6810c20664b1e0ac9/layout/painting/nsImageRenderer.cpp#126

I expect they are the real problem. Even if CANNOT_SUBSTITUTE was set, we don't trigger the factor of 2 scaling since there are only 2 surfaces. But the native size is a factor of 2 of the native size, so it wouldn't prune that one anyways :). It should get evicted when it always does.

I am more annoyed that we are trying to decode ahead of time, and not really use the result because we really want it at a different size. If we requested the decodes later, but fewer of them, the user experience could be just as good. How useful are these signals, really?

Comment 9

[Andrew Osmond [:aosmond] (he/him)]

Oh and:

https://searchfox.org/mozilla-central/rev/38bcf897f1fa19c1eba441a611cf309482e0d6e5/layout/style/nsStyleStruct.cpp#2517