droidstealth / droid-stealth

THIS PROJECT IS DEPRECATED
GNU General Public License v2.0
61 stars 22 forks source link

Thumbnail Caching #183

Closed AlexKolpa closed 8 years ago

AlexKolpa commented 10 years ago

At the moment, the thumbnails are stored in the IndexedFile. These do not get removed once loaded. This will create a problem when having large amounts of files in the app, because Android can't store all those bitmaps in memory simultaneously. It would be better to implement something along these lines: http://developer.android.com/training/displaying-bitmaps/cache-bitmap.html

OlivierHokke commented 10 years ago

we might not want this because the thumbnails should be hidden. but im not sure if this poses a problem with the link you provided. Did not get to read it yet. But the reason it is built like it is now, is because I only allow them to be loaded in memory and they may only exist as files in an encrypted state. A solution would be to not remember the bitmaps and only load those in memory of the items that are VISIBLE, that would mean that when you scroll, the thumbnails have to be reloaded (read: decrypted)

AlexKolpa commented 10 years ago

Yes, that's the entire idea of this system. An LRUCache stores the bitmaps them as weak references, allowing them to be cleaned up when android needs the memory. It will require reloading them from time to time, but prevents OutOfMemoryExceptions.

OlivierHokke commented 10 years ago

I possibly did that already, if not, we can easily change to code to use weak references it's a smaller change Sent using CloudMagic On Tue, Apr 22, 2014 at 8:51 PM, Alex notifications@github.com wrote:Yes, that's the entire idea of this system. An LRUCache stores the bitmaps them as weak references, allowing them to be cleaned up when android needs the memory. It will require reloading them from time to time, but prevents OutOfMemoryExceptions.

—Reply to this email directly or view it on GitHub.

OlivierHokke commented 10 years ago

Good idea though :)

AlexKolpa commented 10 years ago

It never hurts managing memory intensive resources from a single location either, instead of having all those IndexedFiles keeping a reference. Harder to clean up that way.

OlivierHokke commented 10 years ago

Perhaps, but it also increases dependencies and thus complexity. And cleanups should be fine atm. Except possibly the thumbs AFAIK

AlexKolpa commented 10 years ago

Preferably an increase in complexity than such a large increase in memory that it crashes our app :+1:

OlivierHokke commented 10 years ago

You didn't get what I meant apparently. That small change should fix that issue. But if you are going to change it, move the thumb management to the thumbnail manager. That's a better place for the anyway, although when I wrote the current system, I didn't want to create an extra dictionary to manage them.

AlexKolpa commented 10 years ago

I got what you meant, but I'm saying that to manage those thumbnails, the best approach is a centralized one. Android even provides a class for that, as you can see in the link that I placed here. So instead of doing everything yourself, you leave it up to the system to manage it, and only do retrieval when necessary. Saves a lot of coding.

OlivierHokke commented 10 years ago

at this stage it doesn't save a lot of coding (alternatively we could just store them as weakreferences instead of normal ones, just needs 3 lines to be changed), but other than that: you convinced me.

alexwalterbos commented 10 years ago

@OlivierHokke @AlexKolpa since you discussed this quite thoroughly, can you estimate whether we will have daily OOMExceptions b/o this? If so, it can be added to the 25 sept milestone. Also, it should be labeled 'bug' and not 'enhancement', if so.