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

Memory problem or leak on iOS 7 ? #538

Closed
kefon opened this issue Oct 27, 2013 · 128 comments
Closed

Memory problem or leak on iOS 7 ? #538

kefon opened this issue Oct 27, 2013 · 128 comments

Comments

@kefon
Copy link

kefon commented Oct 27, 2013

Hello everybody,
I'm sorry in advance as maybe this is a dumb and noob question...

I'm using SDWebImage to display pictures in a UITableView in my cellForRowAtIndexPath method, using the classic

[cell.pointPicture setImageWithURL:[NSURL URLWithString:thePoint.imageURL] placeholderImage:[UIImage imageNamed:POINT_DEFAULT_IMAGE]];

(the displayed pictures are light and well compressed jpgs, just some ko)
When I inspect my app with "Instrument - Allocations", and just scroll down my UITableView (with 40 cells containing picture, a bit like Instagram), I got a huge amount of memory used ! (see screenshot)

screen

But it seems to be "VM", and especially "VM: CG raster data" from the coreGraphics library.

So the questions are :

  • Is it normal?
  • Is that a serious problem?
  • Is there a way to avoid this?

I'm sorry but after few search on the web I can't find any relevant information concerning the "VM: CG raster data"... Any idea? Thanks in advance !

@ivanbruel
Copy link

I am indeed having the same problem on an iOS7-only app. Images don't seem to be recycled. Seems to be related with [SDWebImageDownloaderOperation connectionDidFinishLoading:] and the ForceDecode of the UIImage.

@kefon
Copy link
Author

kefon commented Oct 28, 2013

@ivanbruel nice to know that I'm not the only one ;) I still looking to understand the problem, but unsuccessfully for now...

@kefon
Copy link
Author

kefon commented Oct 28, 2013

I also have the same issue while running the SDWebImage demo project (on iOS7 / iPhone 5)

screen2

@rs
Copy link
Member

rs commented Oct 28, 2013

SDWebImage cache images is using NSCache. It is discardable memory. See Apple documentation: https://developer.apple.com/library/ios/documentation/cocoa/reference/NSCache_Class/Reference/Reference.html

@kefon
Copy link
Author

kefon commented Oct 28, 2013

@rs Ok ! So if I understand, this is a perfectly normal behavior and the memory is released if needed. Thank you for your answer, now I think I understand what's going on. Sorry for the noob question ! (So I can close this issue right?)

@ivanbruel
Copy link

Well, from my particular issue, I'm URLing into the documents folder, the memory keeps growing no matter how many passes I make through the list, it will most of the time fetch them from disk instead of memory. So does this mean the discardable memory is being discarded even while it grows?

@cloay
Copy link

cloay commented Nov 5, 2013

I have this issue only on iOS7. How to avoid this?

@mrtristan
Copy link

i see this too, are we thinking this isn't an issue? @rs are you saying the memory will grow and grow until it actually needs to be reclaimed?

@rs
Copy link
Member

rs commented Nov 5, 2013

Yes, it's caching

@williamkey123
Copy link

I am seeing a lot of crashes as the CG raster data grows - it doesn't look like it's actually discarding memory when it needs to be. I see memory warnings start to get generated, it kills background apps, and then the app crashes without ever discarding any of the CG raster data.

@phionardo
Copy link

I have exactly the same issue, too. @rs Is there a way to clear cache timely, or reduce using memory for caching?

In my case, the "All Anonymous VM" grows to 700MB and my app crashes, due to the huge amount of "CG raster data".

@rs
Copy link
Member

rs commented Nov 7, 2013

We could play with the totalCostLimit of NSCache to implement a maximum used memory barrier.

@williamkey123
Copy link

I added this:

// Init the memory cache
_memCache = [[NSCache alloc] init];
[_memCache setTotalCostLimit:41943040];
_memCache.name = fullNamespace;

But I'm still getting this as I scroll through a UITableView with lots of images:

screen shot 2013-11-07 at 10 04 50 am

Note the CG Raster Data is still blowing up and it's killing other apps but never reducing the raster data.

Is there something I should be doing to release the images in the table cells when the cells are dequeued and reused? Sorry if that's a stupid question.

@rs
Copy link
Member

rs commented Nov 7, 2013

I guess you have to set the cost of each entry at insertion time otherwise it can't work.

@williamkey123
Copy link

Looks like right now it's setting a cost for each entry of the image height * width * scale as an approximation for image size... I've changed this to:

    NSData *png_representation = UIImagePNGRepresentation(image);
    [self.memCache setObject:image forKey:key cost:png_representation.length];

In the three places where setObject happens - but memory usage is still exploding. I have it capped at 80 MB but CG Raster Data continues to increase unbounded... I suspect this could be the way CoreGraphics is storing rendered images in the view and has nothing to do with the cache that SDWebImage is keeping. But I just have no idea how to tell CG how to free this raster data when it's not on screen.

@rs
Copy link
Member

rs commented Nov 7, 2013

A lot of apps are using SDWebImage caching a lot of images with no memory issue. You may have better chances searching for a leak in the rest of your app yes :)

@williamkey123
Copy link

Definitely possible. I'm not seeing any memory actually leak in Instruments, but I'll keep trying to track it down. Thanks for your responses @rs !

@kaishin
Copy link

kaishin commented Nov 17, 2013

@rs Also reported in #548. This is clearly a leak that has to do with GIFs. Removing GIFs or using AFNetworking which doesn't support GIFs 'fixes' the problem.

@wujie0919
Copy link

I also encountered such a problem

@andrewprocter
Copy link

I'm encountering this as well. I believe @kaishin is onto something re: GIF's.

@pradeepfission
Copy link

I'm encountering ImageIO_PNG_Data memory problem. I'm seeing following in instruments
screen shot 2013-11-28 at 5 43 15 pm

CGRaster Data and ImageIO_PNG_Data are taking a lot of memory.

@jxdwinter
Copy link

👍
Maybe some issues in iOS7 with this GIF stuff. I build a simple project in both iOS6 and iOS7. Check this out.Same code.

iOS6
screen shot 2013-12-02 at 2 04 29 pm

iOS7
screen shot 2013-12-02 at 14 01 49

@myell0w
Copy link

myell0w commented Dec 6, 2013

seeing the same problem and lots of crashes related to SDWebImage. @rs: I think that's an issue worth looking into and isn't related to the affected apps.

@daveluong
Copy link

We're having the same memory problem with SDWebImage too. Seem to happen only on iOS7 with CGRaster Data keep growing to hundred of MBs just by swiping the table view, to the point that the app will get memory warning and be terminated.

I personally believe this got something to do with iOS than the library itself.

@jxdwinter
Copy link

Hi,guys, just use UIWebView to show a gif right now.

@donholly
Copy link
Contributor

How is that related to this topic?

@daveluong
Copy link

Seem like this is a bug in iOS Simulator only. I've just tested on 7.0.3 device and saw a huge drop in memory usage. Looks like the system is claiming back the memory when it's needed.

vw2piczqaiizmpvnm6ptrxym3sm06xqxzu5_xnrq8zy

@kaishin
Copy link

kaishin commented Dec 13, 2013

@Huyduc Are you testing with GIFs? You don't seem to be.

@KetchupMoose
Copy link

I also have this issue, but I am seeing the same problem on a device running ios 6.1 but the ios SDK 7.0.

I am using GIFs.

@Pei116
Copy link

Pei116 commented Jan 7, 2014

I'm facing the same issue. It gets memory warning and finally crashes due to memory pressure when I set image with url.

@burakkilic
Copy link
Contributor

Actually no. My client uploaded them. What is the optimum resolution?

Adil Burak Kılıç

On Wed, May 13, 2015 at 11:13 AM, Konstantinos K. notifications@github.com
wrote:

oh come on...thats huge. Thats why it crashes. Even if your size is 80kb
your images are large in resolution. You should also experience laggy
scroll. Generally speaking you do not add large res images as thumbs in a
tableview/collection view. You add smaller ones. Do you actually need 300
ppi for thumb images?


Reply to this email directly or view it on GitHub
#538 (comment).

@mythodeia
Copy link
Contributor

In my apps i use 300x300 max as thumbs while scrolling. You could use lower if you want.
And you can load the large image when you tap the thumb.
There is no need to make them square like i said but you get my point

@benbonnet
Copy link

@burakkilic "300 Pixel per Inch" is default for print, not digital

@burakkilic
Copy link
Contributor

Thank you. I am downgrading to 72

Adil Burak Kılıç

On Wed, May 13, 2015 at 1:11 PM, bbnnt notifications@github.com wrote:

@burakkilic https://github.com/burakkilic "300 Pixel per Inch" is
default for print, not digital


Reply to this email directly or view it on GitHub
#538 (comment).

@rromanchuk
Copy link
Contributor

Guys, the images you are posting do not show any memory issues. If your apps are lagging it's most likely because you are doing silly things on the main thread. @trunglee you say huge memory growth but what exactly is your persisted memory after scrolling?

@trungp
Copy link

trungp commented May 13, 2015

I have a table view with a list of image urls. When the table scrolling down, it's show image on every row. That's all. For some first pages (about 20 rows per page), the table view is still smooth in an acceptable way. But from the page 3, the table view is going to lagging. I just keep a list of urls as text string for table view metadata and use SDWebImage to load image for imageView on table view cell.

@pajapro
Copy link

pajapro commented Sep 6, 2015

It seams like a long discussing around this particular bug, but is anyone still experiencing a huge growth in memory allocation when loading high resolution images? I experience this issue primary on iPhone 6. I tried the suggested solution (from fork https://github.com/harishkashyap/SDWebImage/tree/fix-memory-issues) to turn off
[[SDImageCache sharedImageCache] setShouldDecompressImages:NO];
[[SDWebImageDownloader sharedDownloader] setShouldDecompressImages:NO];
but that does not solve the memory growth. For other low resolution images I see no significant growth

@bpoplauschi
Copy link
Member

@pprochazka72 are you using static images with high res or animated ones (GIF or other format)?

@pajapro
Copy link

pajapro commented Oct 7, 2015

@bpoplauschi static images with high res.

@CavalcanteLeo
Copy link

+1

@weng1250
Copy link

[[SDImageCache sharedImageCache] setShouldDecompressImages:NO];
[[SDWebImageDownloader sharedDownloader] setShouldDecompressImages:NO]; is good for me to avoid huge memory cost when it is processing GIF format pics.

@bpoplauschi
Copy link
Member

We have replaced our GIF component with FLAnimatedImage. You can try our 4.x branch (instructions).

@songshp39
Copy link

image = [[UIImage alloc] initWithData:data];
This init method will bring in a memory spike when the image data is larger than 1M.

@bpoplauschi
Copy link
Member

Closing. The FLAnimatedImage solution replacing our own GIF is integrated into the 4.x branch.

@bpoplauschi
Copy link
Member

@mythodeia @rs it seems that 3 different people created PR's with the same idea of fixing some of the memory issues we have: scaling down the large images: #787 #884 #894. Do you think it is a good idea? Which one do you prefer? If we merge any of them, I would go with a SDWebImageScaleDownLargeImage option so people can go around it if they really need to.

@bpoplauschi bpoplauschi reopened this Oct 1, 2016
@mythodeia
Copy link
Contributor

mythodeia commented Oct 1, 2016

@bpoplauschi #884 and #894 are almost the same.From these two i prefer #884.
Now for #787 i would like to see some benchmarks and the code needs some cleanup.
However i like the idea of making it optional like you said

@bpoplauschi
Copy link
Member

@mythodeia I agree. Now since I saw that #787 was the most solid PR and it also included the Apple sample for downscaling, I considered including it is best. I merged it via 959d965 (of course it had conflicts and I updated the code). Then I added another commit 00bf467 cleaning stuff up in the Decoder class. Take a look and let me know what you think. We still need to bechmark it.

Everyone, #787 and 00bf467 should give you a good solution for dealing with large images. Together with the GIF handled by FLAnimatedImage, this ticket should be done now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests