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

Disk cache optimization #306

Closed
nebillo opened this issue Feb 21, 2013 · 3 comments
Closed

Disk cache optimization #306

nebillo opened this issue Feb 21, 2013 · 3 comments

Comments

@nebillo
Copy link
Contributor

nebillo commented Feb 21, 2013

hello,
I was playing with your code, looking for some inspiration for writing my own disk image cache. I think you did an awesome job, but I am still missing something...

  • you are cleaning the disk cache when the system fires a UIApplicationWillTerminateNotification notification. based on apple doc this notification might be sent just in some circumstances, or it might not be called at all like it is in devices without multitasking. you should instead be listening for UIApplicationDidEnterBackgroudNotification.
  • even if you are performing the IO operation on a separate queue, the items to remove might be a lot and the operation it self could takes long long time, definitely more than the few seconds apple gives you to complete you task. why not taking advantages of the 10 more minutes by implementing a long background task?
  • since you need to read the content-modification-date of every files, there's a faster and more efficient way of prefetching this property during the enumeration, instead of performing an additional IO operation for every file. you just need to refactor you code and initialize the enumerator with the method enumeratorAtURL:includingPropertiesForKeys:options:errorHandler:
    if you want I can give you more details on this with a working example.

I'm wondering if you have already considered and ignored these things, and in this case it would be interesting to know why.

@rs
Copy link
Member

rs commented Feb 21, 2013

First two points are so because the code pre-dates multi-tasking support in iOS. We should clearly change the notification name and perform a background task, you're 100% right.

The last point is perfectly valid as well. Would you consider a pull request?

@nebillo
Copy link
Contributor Author

nebillo commented Feb 21, 2013

pull request for point 3 done :)

@bpoplauschi
Copy link
Member

I think all the points suggested by @nebillo are solved:

  • listening to both UIApplicationWillTerminateNotification and UIApplicationDidEnterBackgroundNotification for cleaning the disk cache
  • using a background task in [SDImageCache backgroundCleanDisk]
  • using the enumeratorAtURL:includingPropertiesForKeys:options:errorHandler: API Optimizing enumeration of files while cleaning the disk cache #307

devedup pushed a commit to FilmFlexMovies/SDWebImage that referenced this issue Sep 10, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants