2

I am trying to create cached image system for Android but the memory consumption just grows and grows. I looked through Android website for some ideas, but the issue just doesn't want to disappear.

Below is my code of getting the image from SD card, setting it and later destroying. What am I doing wrong?

WeakReference<Bitmap> newImageRef;
    public void setImageFromFile(File source){
        if(source.exists()){

            Bitmap newImage = BitmapFactory.decodeFile(source.getAbsolutePath());
            newImageRef =   new WeakReference<Bitmap>(newImage);
            if(newImage != null){
                this.setImageBitmap(newImage);
            }
        }
    }

    @Override
    protected void onDetachedFromWindow() {
        Bitmap newImage = newImageRef.get();
        if (newImage != null) {
        newImage.recycle();
        newImage = null;
        }


        Drawable drawable = getDrawable();
        if (drawable instanceof BitmapDrawable) {
            BitmapDrawable bitmapDrawable = (BitmapDrawable) drawable;
            Bitmap bitmap = bitmapDrawable.getBitmap();
            if (bitmap != null){
            bitmap.recycle();
            }
        }
        this.setImageResource(0);
        newImage = null;
        newImageRef = null;
        System.gc();


        super.onDetachedFromWindow();
    }
Arturs Vancans
  • 4,531
  • 14
  • 47
  • 76
  • I'm surprised it doesn't crash. You essentially recycle the same bitmap twice (once as newImageRef, then as drawable). Are you sure onDetachedFromWindow is called? Did you trace it? – msh Jun 10 '13 at 01:27
  • @msh Well I have `if (bitmap != null){` in the second cycle, so it doesn't crash. And yes, that method is called. I tried to get a heap dump and it says that Bitmap and BitmapDrawable takes the most memory. – Arturs Vancans Jun 11 '13 at 11:49
  • Bitmap.recycle() doesn't make it null, it deallocates internal bitmap storage. If you deallocate it twice you may end up with crash. – msh Jun 11 '13 at 17:05
  • @msh well. in any case. even if i recycle only once. the memory is still growing. :( – Arturs Vancans Jun 11 '13 at 18:16
  • 2
    why don't you use lazy laoding. http://stackoverflow.com/questions/16789676/caching-images-and-displaying/16978285#16978285. instead of urls use the path of the iamges from sdcard – Raghunandan Jun 20 '13 at 07:32
  • 1
    Try to also call `Drawable.setCallback(null);` – Sherif elKhatib Jun 20 '13 at 09:48
  • Do not keep long-lived references to a context-activity. Try using the context-application instead of a context-activity Avoid non-static inner classes in an activity if you don't control their life cycle, use a static inner class and make a weak reference to the activity inside. The solution to this issue is to use a static inner class with a WeakReference to the outer class, as done in ViewRoot and its W inner class for instance A garbage collector is not an insurance against memory leaks – Diogo Bento Jun 20 '13 at 14:59
  • 5
    I would really use https://github.com/square/picasso or https://github.com/novoda/ImageLoader and avoid reimplementing the wheel. – Ovidiu Latcu Jun 21 '13 at 09:47
  • Are you running above code while scrolling on list view? If yes kindly have a check on your adapter if you're using any convert views or just inflating on each getView, that would increase your memory consumption too. – Chor Wai Chun Jun 22 '13 at 15:19
  • When you used MAT to see where your memory is going, what did you learn? – CommonsWare Jun 22 '13 at 21:48
  • Raghunandan Ovidiu , I cannot use use any 3rd party libraries. CommonsWare , Bitmap and BitmapDrawable as far as I remember consumed 80% of the mem. – Arturs Vancans Jun 23 '13 at 09:22
  • Chor, I am using the recycled views. – Arturs Vancans Jun 23 '13 at 09:23
  • how do you know that you have a memory leak? – Blackbelt Jun 26 '13 at 12:10
  • @blackbelt because after destroying the activity, the memory does not decrease and eventually throws "out of memory" error. – Arturs Vancans Jun 26 '13 at 20:08
  • how big (widht and height) those images are? – Blackbelt Jun 26 '13 at 20:09
  • @blackbelt 400KB to 1MB and around 6 images in total – Arturs Vancans Jun 26 '13 at 22:32

3 Answers3

2

If you are using Android version >3.0 you dont have to call recycle()as the gc will clean up bitmaps on its own eventually as long as there are no references to it. So it is safe to remove recycle calls. They do nothing much here.

The code which you posted looks neat but are you sure there the leak is not happening somewhere else. Use Android Memory Analyzer tool to see where the leak is happening and then post the info.

Good luck.

blganesh101
  • 3,647
  • 1
  • 24
  • 44
1

Try to use Drawable.setCallback(null);. In Android 3.0 or newer, you don't even need to recycle because of more automatic memory management or garbage collection than in earlier versions. See also this. It has good information about bitmap memory management in Android.

user2448027
  • 1,628
  • 10
  • 11
0

As of this code it's hard to check if there is a detailed bug as this seems to cleary be a simplifyed version of the "full cache". At least the few lines you provided seem to look ok.

The main issue is the GC seems to be a little strange when handling Bitmaps. If you just remove the hard references, it will sometimes hang onto the Bitmaps for a little while longer, perhaps because of the way Bitmap objects are allocated. As said before recycling is not necessary on Android 3+. So if you are adding a big amount of Bitmaps, it might take some time until this memory is free again. Or the memory leak might be in anothe part of your code. For sophisticated problems like that its wise to check already proven solutions, before re-implementing one.

This brings me to the second issue: the use of weak refrences. This might not target the main problem, but is generally not a good pattern to use for image caches in Android 2.3+ as written by android doc:

Note: In the past, a popular memory cache implementation was a SoftReference or WeakReference bitmap cache, however this is not recommended. Starting from Android 2.3 (API Level 9) the garbage collector is more aggressive with collecting soft/weak references which makes them fairly ineffective. In addition, prior to Android 3.0 (API Level 11), the backing data of a bitmap was stored in native memory which is not released in a predictable manner, potentially causing an application to briefly exceed its memory limits and crash.

The way to go now is to use LRU Caches, which is explained in detail in the link provided about caching.

Patrick
  • 33,984
  • 10
  • 106
  • 126