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

Possible Memory Waste (OutOfMemoryError) with Solution #475

Closed
chiavat opened this issue Dec 5, 2013 · 4 comments
Closed

Possible Memory Waste (OutOfMemoryError) with Solution #475

chiavat opened this issue Dec 5, 2013 · 4 comments
Labels

Comments

@chiavat
Copy link

chiavat commented Dec 5, 2013

Hi,

I am using this library since 3 days and using the dump profile of eclipse I found that if application uses ImageLoader in a SurfaceView, there could be a Possible Memory Waste

This is the dump in my app:

Duplicate Strings (9.516.384 total bytes wasted Instead of 192 total bytes):

Top elements include:
•94.420 × assets://used_always_image.png_480x320\u0000\u0000\u000... (96 bytes)
•4.709 × assets://used_less_image.png_480x320\u0000\u0000\u0000... (96 bytes)

The real problem is in com.nostra13.universalimageloader.core.ImageLoaderEngine and exactly in:

void prepareDisplayTaskFor(ImageAware imageAware, String memoryCacheKey) {
  cacheKeysForImageAwares.put(imageAware.getId(), memoryCacheKey);
}

Step to reproduce:

  1. Create a View that extends SurfaceView and create the SurfaceThread class
  2. Insert the following code in SurfaceThread class to draw the images:
private static final int FRAME_DELAY = 50;
private volatile boolean requestStop;
private long executionTime, sleepTime;

public synchronized void requestStop() {
    this.requestStop = true;
}

public SurfaceThread() {
    super();
    this.setName("SurfaceThread");
}

@Override
public void run() {
    while (!requestStop) {
        Canvas canvas = null;
        Random rnd = new Random();
        try {
            executionTime = SystemClock.elapsedRealtime();

            canvas = surfaceHolder.lockCanvas();
            synchronized (LOCK_DRAWINGS) {
                Bitmap bmp = ImageLoader.getInstance().loadImageSync("assets://always_used_image.png");
                canvas.drawBitmap(bmp, 0, 0, null);
                if (rnd.nextInt(100) > 90) {
                    bmp = ImageLoader.getInstance().loadImageSync("assets://less_used_image.png");
                    canvas.drawBitmap(bmp, 0, 0, null);
                }
            }
        } catch (Exception ex) {
            if (BuildConfig.DEBUG) {
                //Will enter if the bitmap loaded is null but will crash for OutOfMemoryError
                Log.e("GameThread", ex.toString(), ex);
            }
        } finally {
            if (canvas != null) {
                surfaceHolder.unlockCanvasAndPost(canvas);
            }

            sleepTime = FRAME_DELAY - (SystemClock.elapsedRealtime() - executionTime);
            try {
                if (sleepTime > 0) {
                    Thread.yield();
                    Thread.sleep(sleepTime);
                }
            } catch (InterruptedException e) { }
        }
    }
}
  1. In Eclipse click Devices->Update Heap
  2. In Eclipse click Devices->Dump HPROF File
  3. In dump check for "Top Components" or "Leaks Suspect"
  4. Repeat steps 4 to 6 every 30 seconds
  5. You will notice an increase in memory for the object cacheKeysForImageAwares
@chiavat
Copy link
Author

chiavat commented Dec 5, 2013

I fixed your source code in this way:

public class ImageNonViewAware implements ImageAware {
    protected final String uri;
    protected final ImageSize imageSize;
    protected final ViewScaleType scaleType;

    public ImageNonViewAware(String uri, ImageSize imageSize, ViewScaleType scaleType) {
        this.uri = uri;
        this.imageSize = imageSize;
        this.scaleType = scaleType;
    }

    @Override
    public int getId() {
        return TextUtils.isEmpty(uri) ? super.hashCode() : uri.hashCode();
    }
}
public class ImageLoader {
    public void loadImage(String uri, ImageSize targetImageSize, DisplayImageOptions options,
                          ImageLoadingListener listener) {
        checkConfiguration();
        if (targetImageSize == null) {
            targetImageSize = configuration.getMaxImageSize();
        }
        if (options == null) {
            options = configuration.defaultDisplayImageOptions;
        }

        ImageNonViewAware imageAware = new ImageNonViewAware(uri, targetImageSize, ViewScaleType.CROP);
        displayImage(uri, imageAware, options, listener);
    }
}

Now there is no waste of resource and this library is great. No more OutOfMemoryError

@nostra13
Copy link
Owner

nostra13 commented Dec 5, 2013

I got your fix. Actually I thought about exactly the same version of ImageNonViewAware before release but I decided to left the current version because if we use your ImageNonViewAware version (ImageNonViewAwares with the same URI have the same IDs, i.e. they are considered as equals) and you load images asynchronously (loadImage(...)) then the second call (or every other following call) with the same image URI will cancel previous loading task for that image. There was no such behavior in previous versions so I decided to choose current ImageNonViewAware version.

You have a very special usage of ImageLoader, you call loadImageSync() many many times so you got OOM. UIL doesn't ready for that :) I'll think about your fix.

@chiavat
Copy link
Author

chiavat commented Dec 5, 2013

I use in this way because I need to use the last recents cached bitmaps if exist in memory cache or disc cache, else load and cache image (i do some long operation on the image the first time before cache)

I arrived at this solution because I noticed that if I load an image with an uri, the uri is unique to the image and will be unique even then the hashCode. So there is no need to cancel previous loading task.

Ex: hashCode-uri
5548545-assets://thumb_128x128.png
4384783-assets://thumb_64x64.png
8473411-http://www.foo.com/foo.png
8555341-http://www.foo.com/foo_small.png

Instead, when I use the method displayImage, the ImageViewAware works well because it uses imageView.hashCode as id.

PS: my app is a game with animations and these are the results after the fix:
Before FIX: after 30 minutes Dump HPROF File shows UIL=20Mb in Heap
After FIX: dump profile Dump HPROF File shows 2mb in Heap (ThreadPool, HttpConnection, and other android classes)

@nostra13
Copy link
Owner

nostra13 commented Dec 5, 2013

Yes, I understand. You should use ImageNonViewAware 2.0 in your case.

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

2 participants