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

Enhance thumbnail downloading / Download all thumbnails in one folder / smart cache size #177

Open
tobiasKaminsky opened this issue Jul 30, 2016 · 19 comments
Labels
enhancement feature: previews and thumbnails performance 🚀 Performance improvement opportunities (non-crash related)

Comments

@tobiasKaminsky
Copy link
Member

Download all thumbnails in one folder with one click -> three dots menu.
@jancborchardt should there be a notification for this which displays the progress?

@jancborchardt
Copy link
Member

That should be done automatically on entering the folder and not require an extra action. Or which thumbnails do you refer to?

@Spacefish
Copy link
Member

No, Thumbnails are only downloaded once you scroll down.. the disk cache has a limited size 8 Mbytes as far as I remember and the purge policy is LRU. There might be folders which contain more than 8 MByte of thumbnails! Even if not, once you reach the 8 mb threshold, thumbs from other folders are evicted from the cache.

@tobiasKaminsky
Copy link
Member Author

This is a tricky one, I think...
As @Spacefish pointed out we have a limited (currently 10mb) cache with LRU.
In the next version it will be possible to resize the cache to any value.

The reason for the idea of downloading all thumbnails is that otherwise one have to scroll slowly through all images in one folder.
@jancborchardt proposal is problematic when you are on a limited network and it takes up all network bandwith.

With these both restrictions (bandwith and cache size) I thought it must be an optional action that the user have to choose (and therefore really want it).

PS: A thumbnail is ~15kb.

@strugee
Copy link
Member

strugee commented Aug 2, 2016

We should always download all thumbnails on (non-metered?) WiFi. Otherwise I think the best design would be to download all thumbnails on screen, and a certain number off the bottom and top, in case the user scrolls.

@jancborchardt and others, thoughts?

@AndyScherzinger
Copy link
Member

Download behavior

How about making it depend on the connection:

  • metered wifi / cellular network: keep the actual download behavior for thumbnails
  • unmetered wifi: download them all

Cache size

Could we make this dynamic and choose a cache size during the first start of the app and maybe every once in a while?

  • calculate cache size as: 5% of the available space but max 100MB?
  • recalculate cache size: If possible check the available space every 4 weeks and recalculate as described above

Just an idea. So.. @ all : Would this be possible? And would this make sense?

@strugee
Copy link
Member

strugee commented Aug 2, 2016

@AndyScherzinger sounds similar to what I said; +1

@AndyScherzinger
Copy link
Member

yes, I basically summed up the post and comments :)

@jancborchardt
Copy link
Member

@strugee @AndyScherzinger sounds good. No options, just automatic and fitting behavior. :)

@Spacefish
Copy link
Member

Spacefish commented Aug 2, 2016

I wouldn't limit the Cachesize to 100mb. I would go with 5% and only if the remaining free storage is <80% of the total device storage limit it to the said 100mb.
If you look at the Google Fotos App, it will take multiple 100mb of Thumbs.

@AndyScherzinger
Copy link
Member

AndyScherzinger commented Aug 2, 2016

@jancborchardt @tobiasKaminsky I think the simpler version as described by @Spacefish by simply going for 5% as long as a certain device storage is still available would be best. I would change it a little though and say:

  • >=40% of the total storage available: set cache size to 5% of total storage size of the device
  • < 40% of the total storage available: set cache size to 10% of the remaining space available but at least 20MByte (which still gives you space for >1300 image thumbnails)

sounds a bit complicated but should work nicely :) What do you think @tobiasKaminsky ? Hope it is not to over-engineered 😋

@tobiasKaminsky
Copy link
Member Author

Download behaviour is fine for me. But we have to ensure that it does not delay any other task, e.g. downloading a file or fetching a resized image.

Cache size: I would not set any limit, but increase it dynamically.
The cache size will also be adjustable with #144
So the current maximum should be displayed there with an information that it is increasing automatically.
If a user sets a value this will be a hard maximum which has to be respected. If it is too low (according to @AndyScherzinger metric or any else) there should be a warning and a proper proposal.

@AndyScherzinger
Copy link
Member

There should imho also be a hard maximun, simply to be a good citizen on the phone. 😃

@tobiasKaminsky
Copy link
Member Author

How are other apps handling this? Dropbox, etc?

@AndyScherzinger
Copy link
Member

I don't know since this is an internal implementation, blackbox... :(

@tobiasKaminsky
Copy link
Member Author

Na, I meant just be downloading lots of dropbox content and see the cache size ;)
I do not have a dropbox...

@jancborchardt
Copy link
Member

@tobiasKaminsky just create a fakeaccount for testing purposes, that’s always recommended to check what others are doing. :)

@tobiasKaminsky tobiasKaminsky changed the title Download all thumbnails in one folder Enhance thumbnail downloading / Download all thumbnails in one folder Jan 28, 2017
@tobiasKaminsky
Copy link
Member Author

I have a new idea:
Initial sync: Based on the timestamp of folder (and its content) we can go from new to old and download all thumbnails. This can stop when cache limit is reached. So we make sure that the most recent images have a thumbnail.

Regular sync: Based on the same logic of above we can do a regular sync. As we have a "least recently used" cache policy all new thumbnails and most often used thumbnails should stay in cache.

Both can be done automatically via a JobScheduler with the limitation "wlan" and "charging".

@strugee
Copy link
Member

strugee commented Jan 28, 2017

@tobiasKaminsky that sounds like a wonderful idea. We should still implement the previously discussed behavior though, when actively browsing to a folder.

@tobiasKaminsky tobiasKaminsky changed the title Enhance thumbnail downloading / Download all thumbnails in one folder Enhance thumbnail downloading / Download all thumbnails in one folder / smart cache size Mar 3, 2017
@jancborchardt
Copy link
Member

Good call! So the previously mentioned logic should be used, preferring the generation of previews for recently modified files. :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement feature: previews and thumbnails performance 🚀 Performance improvement opportunities (non-crash related)
Projects
None yet
Development

No branches or pull requests

6 participants