Disk Cache can grow significantly larger than 'sizeLimit'
hotngui opened this issue · 4 comments
Check List
Thanks for considering to open an issue. Before you submit your issue, please confirm these boxes are checked.
- [x ] I have read the wiki page and cheat sheet, but there is no information I need.
- [x ] I have searched in existing issues, but did not find a same one.
- [x ] I want to report a problem instead of asking a question. It'd better to use kingfisher tag in Stack Overflow to ask a question.
Issue Description
I was doing some stress testing on devices whose disks are almost full. I noticed that disk cache can grow significantly larger than what I specify via the ImageCache.default.diskStorage.config.sizeLimit until the app is backgrounded or terminated.
What
I took a look at the source code, but could not find anywhere that checks the disk space used by the KF cache against the sizeLimit to keep the cache from growing too large while the app is still in the foreground.
Am I missing some configuration flag or something?
Reproduce
This can be tricky as you need to have a device whose disk is almost full - from another app that is not using the OS cache sandbox since that would not work correctly.
The current behavior is intentionally designed to check and remove cache files that exceed the size limit only when the app enters the background.
This approach prevents performance issues that could arise from checking the file size every time a new image is cached, which could significantly impact performance.
If needed, you can manually invoke the removeSizeExceededValues method on the disk cache at any time to remove files that exceed the specified size limit.
Yes, I understand the current behavior was intentional and that it's not practical to check each time. I was thinking more around the idea of call removeSizeExceededValues when you did receive an error on attempting to write to the disk cache or something.
I was not able to find a central location where I could attempt to catch the error myself; or a closure I could register to be called only when that error got thrown. Perhaps I missed something in the code?
I work on several apps that can be heavy on images in long user sessions, so having the cache grow with limits during a single session can be an issue.
Currently, there actually is an error defined in Kingfisher when writing disk cache failed, as the CacheErrorReason.cannotCreateCacheFile, which is thrown in the ImageCache as in CacheStoreResult.diskCacheResult. However, since when using the KingfisherManager and the view extension methods, the cache process happens in an async way (by default, after the image setting completion handler is called) and will not affect the UI layer, this error is ignored (case 1, case 2).
If you need to implement a way to inspect this, maybe you can try to add a delegate to expose this error out and perform a clean up work there.
Or easier, maybe you can just set up a timer (say perform it every 5 or 10 minutes) to clean it regularly before an issue may happen.
Or easier, maybe you can just set up a timer (say perform it every 5 or 10 minutes) to clean it regularly before an issue may happen
Yeah, this is what I was thinking as well. But wanted to make sure I was not missing a feature that already existed before going that route as it does feel a bit janky.