Open joshzana opened 9 years ago
Thanks for the report. This is not just for gif, it's looks like those objects are used for every Glide...into()
call.
Is 880B really a big deal? This is a genuine question on my part, and in the following I try to answer myself. Feel free to contradict my reasoning: Let's pick a random size: 500x500, a transparent image of that size is 1MB. Now the garbage for an image that size is 0.088% of a single frame. Looking at a gif I think you'll need 2 such big bitmaps in the pool, because the frames are recycled, so there's no allocation there, just 2MB in Tenure space. To reach the size of a single frame in garbage you would need 1136 frames, which takes 37 seconds at 30fps, but we know gifs aren't that fast :) Let's say the Eden space is 1MB (I couldn't find anything on the matter for android, but I think it's a fair assumption) and with that eden size the garbage will fill in up in roughly 2 minutes if you're just watching a single animated gif of 10fps. I think it's also fair to assume that a user will never stay put for more than 5 seconds, at least according to my usage (even when reading mail), and if they leave the app open on the desk it will sleep eventually so the animation is stopped.
On the other hand let's say every object you mentioned is pooled, I think overhead of managing the pools (mostly in terms of reading/writing code) doesn't worth the effort plus to pools may hold on to objects indefinitely even if the part of the app that's been using images is left and the user navigates to another part. And if WeakReferences are used the bitmap allocation on loading images (not neccessarily gifs) will evict them anyway because the bitmaps are so much bigger. So the pool would only be a short term pool: say <1 second when slowly scrolling down a long list of different sized images.
Thanks for the detailed reply! 880B in isolation is not much, I agree. The problem is that the repeated garbage generation leads to GCs, leading to frame drops. I also agree that I'm not getting 60fps rendering on GIFs anyway, so the additional drops due to GC aren't a showstopper.
The GenericRequest object is interesting to me since it seems that care was taken to add a REQUEST_POOL there, but then there are very few calls to recycle() to fill that up. Is it really expected that repeated requests to decode frames of the same request bypass this pool, when there are so many other pools along the way that are not bypassed? (Byte arrays, GifDecoders, GifHeaderParsers, etc)
Your second paragraph sounds like a bug/enhancement.
It's strange that with so much care put into GifFrameLoader
to handle clearing it's actually not recycled.
I think GifFrameLoader.DelayTarget
is missing an onLoadCleared
override OR ClearTarget
is missing one, OR GenericRequest.clear()
should also recycle()
? It feels like no-one really owns the request. A recycle()
here an = null
there, a clear()
everywhere. @sjudd is the expert in this regard :) Let's see what he says.
I think it may worth re-thinking together with #376?
Yeah there's some work to be done here.
Part of the problem is that we want each gif frame to be loaded independently because the GifDecoder requires that each frame be loaded in sequence so that previous frames can be drawn onto by subsequent frames. Glide doesn't really support this, so we use a hack (UUID signature) to create a unique key.
We really ought to try to find a way to support unique loads in Glide so we don't have to do this. Doing so would eliminate most of the garbage.
I'm currently experiencing a very similar issue using Glide 4.3.1. It seems like every time the Gif (~250kB) loops around the memory climbs and continues to climb despite no interaction with the device. Tested just loading a bitmap instead of a gif and the issue goes away.
@jcromp increasing memory usage sounds like a different issue, though you can use an allocation tracker/heap dump to be sure.
In my app I get a GifDrawable from GifDrawableLoadProvider by giving it a file path. The animation then does the invalidate->draw loop until the back button is pressed.
If I watch the Android Studio memory monitor window, I see the Allocation size steadily increasing. If I grab an allocation tracker dump, I see this pattern repeat every frame:
That's 880 bytes from within each call to GifFrameLoader.loadNextFrame.
One bit of low hanging fruit seems to be the construction of a new GenericRequest object each time (208 bytes). The pool is consulted but is always empty, because rather than recycle these objects, they are cleared and disposed of in GifFrameLoader.FrameLoaderCallback's MSG_CLEAR branch.
Another big block comes from onSizeReady, which allocates 464 bytes in small objects each time as well.
Ideally, an Animated GIF could be played forever with no per-frame draw calls, though I understand that sometimes there are required.