Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Gif memory usage #690

Closed
thest1 opened this issue Oct 17, 2015 · 9 comments
Closed

Gif memory usage #690

thest1 opened this issue Oct 17, 2015 · 9 comments
Labels
Milestone

Comments

@thest1
Copy link

thest1 commented Oct 17, 2015

I have an Activity and I want to display gif in it. I do it using glide 3.6.1. Works just great.

I can see glide uses a reasonable amount of memory to display gif. Mostly it's GifDecoder.rawData. That's OK, I realize we need it.

But when I close the Activity I wan't all this stuff to be garbage collected. Looks like it doesn't happen. rawData still consumes a lot of memory. Do I do something wrong? Or I should do something to free internal resources when I close the Activity and don't want GifDecoder to retain in memory?

I use Memory Monitor in Android Studio. I can see that allocated memory is never released. Gif can be a few megabytes size. Allocating them forever can cause pretty bad OutOfMemory issues.

I use Samsung Galaxy S3 Android 4.3 but I'm pretty sure it works the same for any other device.

@TWiStErRob
Copy link
Collaborator

You can force Glide to clear the target via:

Glide.clear(imageView);

but that should happen automatically where Fragments are available. What did you pass to Glide.with when loading the GIF into the ImageView? Feel free to share your full Glide load line.

@thest1
Copy link
Author

thest1 commented Oct 17, 2015

I pass Activity to Glide.with(), no fragments involved.

Glide.with(this)
.load(file_path)
.diskCacheStrategy(DiskCacheStrategy.NONE)
.skipMemoryCache(true)
.into(imageView);

I tried Glide.clear(imageView) on Activity close, no effect. Still rawData remains allocated.

I thought you're aware about the problem. If you're not I need to ensure I don't miss anything, could just be my fault.

Looks like some GifResourceDecoder.DECODER_POOL references GifDecoder. That's why GifDecoder is retained forever and huge rawData inside it.

Can't see GifResourceDecoder.DECODER_POOL in the latest sources, probably the issue is already fixed? I build against 3.6.1.

@TWiStErRob
Copy link
Collaborator

I pass Activity to Glide.with(), no fragments involved.

I wanted to know this because it means that Glide can add its own viewless [Support]RequestManagerFragment to the activity which in turn means that the imageView should be cleared upon destroy.

Sadly I'm not familiar with the 4.0 codebase yet, it's possible that the pool was just moved to another class or renamed. Did you customize 3.6.1 or it's just a jar or compile dependency?

@TWiStErRob TWiStErRob added the bug label Oct 17, 2015
@TWiStErRob
Copy link
Collaborator

Actually, by a quick glance at GifDecoder it simply doesn't clear rawData, it will wait for that buffer to be GC'd only when the decoder is next used. If you check 4.0 GifDecoder changed a lot and that one nulls rawData in clear().

@thest1
Copy link
Author

thest1 commented Oct 18, 2015

Glide can add its own viewless [Support]RequestManagerFragment to the activity which in turn means that the imageView should be cleared upon destroy.

I see. Probably GifDecoder is not cleared even when ImageView is cleared.

I don't customize anything, I just add "compile 'com.github.bumptech.glide:glide:3.6.1'"

I'll provide issue template, just give me some time.

it will wait for that buffer to be GC'd only when the decoder is next used

Can confirm that. I can see that Gif1 memory is released only when I display another Gif2. If I never display another gif then memory seems to be never released.

If you check 4.0 GifDecoder changed a lot and that one nulls rawData in clear().

Have just tried the latest snapshot build from master. No effect, still the same problem.

@TWiStErRob
Copy link
Collaborator

I found a really hacky way that may allow for further confirmation and maybe as a temporary workaround. It is untested and may interfere with Glide internals (e.g. when using a normal Glide.with.load.into without any as*() calls).

.asGif()
.decoder(new Issue690GifResourceDecoder(context))
package com.bumptech.glide.load.resource.gif; // need to be in same package to access package private stuff

import android.content.Context;

import com.bumptech.glide.Glide;
import com.bumptech.glide.gifdecoder.*;

public class Issue690GifResourceDecoder extends GifResourceDecoder {
    private static final GifHeaderParserPool PARSER_POOL = new GifHeaderParserPool(); // package private class
    private static final GifDecoderPool DECODER_POOL = new Issue690GifDecoderPool();
    public FixedGifResourceDecoder(Context context) {
        super(context, Glide.get(context).getBitmapPool(), PARSER_POOL, DECODER_POOL); // package private ctor
    }

    static class Issue690GifDecoderPool extends GifDecoderPool { // package private class
        private static final byte[] NO_DATA = new byte[0];
        private static final GifHeader EMPTY_HEADER = new GifHeader();
        @Override public synchronized void release(GifDecoder decoder) {
            decoder.clear(); // clear
            decoder.setData(EMPTY_HEADER, NO_DATA); // set dummy data
            super.release(decoder); // clear again so only dummy data is leaked
        }
    }
}

@TWiStErRob
Copy link
Collaborator

@thest1 it could be fixed in 3.7.0-SNAPSHOT from build 5 via #892, you can give it a try.

@sjudd
Copy link
Collaborator

sjudd commented Jan 14, 2016

A similar fix to the one in that PR made it into master at some point as well: https://github.com/bumptech/glide/blob/master/third_party/gif_decoder/src/main/java/com/bumptech/glide/gifdecoder/GifDecoder.java#L417

@TWiStErRob
Copy link
Collaborator

Oh, true that, I said the same above: "If you check 4.0 GifDecoder changed a lot and that one nulls rawData in clear().". I must've confused this issue with something else.

@TWiStErRob TWiStErRob added this to the 3.7.0 milestone Jan 21, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants