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

Improvement - enable caching #35

Closed
MFlisar opened this issue Nov 11, 2015 · 13 comments
Closed

Improvement - enable caching #35

MFlisar opened this issue Nov 11, 2015 · 13 comments

Comments

@MFlisar
Copy link

MFlisar commented Nov 11, 2015

It would be nice if I could enable a cache, so that I can fastly recheck a setting without reading the settings file again.

A simple HashMap would be enough and an annotation like @SharedPreferences(..., enableCaching= true). And if the user enables caching, a static HashMap is always kept in sync when writing values and reading is done by accessing the HashMap...

@dkunzler
Copy link
Owner

Hi,

thanks for your suggestion.

That would only make sense when automatically registering a PreferenceListener on these cached Preferences to circumvent the possibility that values were written outside of esperandro. But with such a Listener it could be possible.
It would however increase the memory footprint of the app and there should definately be an LRU eviction algorithm to make sure not the whole preferences are stored in memory.

But I will consider adding this into the next version.

Any thoughts on the usability of this @stonecs ?

@MFlisar
Copy link
Author

MFlisar commented Nov 11, 2015

Maybe it's even possible to cache selected values with a per preference annotation. As long as you can enable/disable the cache globally when creating the preference class, it an improvement for sure as it does not break nor slow down nor increase the memory footprint for all those, who do not enable this feature.

I'm using your library in some of my projects already and it's not the first time this feature would be useful for me.

Use case for me is following:
I have some global user settings and those settings are used in adapters, lists, views and so on. And I iterate over a lot of items and always check a preference. This takes some time if the value is always retrieved from the settings file. I can solve that with my own cache or with always using local variables, but it just looks better if I just can recall my preferences directly and rely on it's internal caching. And mostly I have a literally irrelevant amount of preferences so I mostly don't mind if all preference values are cached when they are retrieved...

I personally don't think that it's necessary to listen to changes from outside. If i use a library with a cache it's ok for me, that I have to keep it in sync if I do not use the library for what it's used for. Of course that would be some extra functionality though that's useful...

@dkunzler
Copy link
Owner

Hi @MFlisar , i added caching to a preference interface using the @Cached Annotation. I will update the documentation in a few days or so when I have time.
Caching is enabled for whole prefence interface. cacheOnPut = true will update the cache on a put. Otherwise the entry for the key is deleted from the cache.
support = true will use the LruCache from the Support library.
cacheSize should be self-explanatory ;)

If you have any questions, comments or suggestions feel free to post.

@MFlisar
Copy link
Author

MFlisar commented Dec 28, 2015

Do I understand that right? You may use that for your docs if you want to of course ;-)

cacheOnPut
If enabled, any already cached value will be instantly replaced in the cache, whenever it is changed in the sense of whenever a new value is saved. Otherwise, if you save a new value into a preference field, the corresponding evenutally cached value will be removed from the cache.

cacheSize
This is the number of values the cache is able to hold

@danielneu
Copy link
Collaborator

Sorry for being so quiet the last days / weeks, it had been pretty busy here.

I think this is a feature which needs thorough thoughts by developers using it, as prefs might be changed via the normal Android methodology and not via Esperandro. This may end up in wrong behavior, wrong information or even alternating behavior (Esperandro reads different values via cache as are actually stored in the prefs).

One scenario where this might be pretty common is preferences which are written by a PreferenceFragment / Activity. This will always be done without using Esperandro by the Android system and in this case the cache needs to be cleared or updated.

I have to admit, I'm no fan of this feature, because it is simply not possible to know all scenarios where the cached information might be out-dated. Can we add a prominent "caution" section, to raise awareness?

Best wishes
Daniel

@MFlisar
Copy link
Author

MFlisar commented Dec 28, 2015

For me, I never use PreferenceActivities or PreferenceFragments, I have my own implementation for such things... So I'm always using the preferences myself only...

@danielneu
Copy link
Collaborator

Yes, but this is a library not meant for special use-cases but for general purposes. There are PreferenceFragments and they are used (although I also don't like them myself), so we need to think about it.

Having a user change his preferences and not seeing this in action because of wrong cached values is a big issue and can cause a lot of trouble.

@MFlisar
Copy link
Author

MFlisar commented Dec 28, 2015

I understand your concerns, a warning is definetely a good thing, but as this is just an extension I think it's a good one. Of course people have to think about how and when to use it...

@dkunzler
Copy link
Owner

You are right, it is a dangerous feature.
Perhaps I could add a compiler warning or even error if the @Cached Annotation ist used on default SharedPreferences as opposed to named ones.
Since only default Preferences can be used with PreferenceFragment/Activity that should be enough as a warning for a good developer ;)

@danielneu
Copy link
Collaborator

This is what I was thinking about as well, but I found it to be too hard to explain, I didn't think about compiler warnings and errors, though.

Do you in the meantime see compiler warnings in Android Studio? IT#s been more than a year since I wrote my last line of code in it...

If not (or even if) I would go for an error and only allow it in named preferences. We could also think about adding an option to override this, but then we know people are doing it after dealing with this topic.

Best wishes
Daniel

@MFlisar
Copy link
Author

MFlisar commented Jan 21, 2016

Could you please add one additional feature:

A setting for auto calculation of cache size to cache ALL values? Or a percentage base setup so that I can setup something like cacheSizeInPercentages = 100 so that the size is calculated based on the number of preferences?

@dkunzler
Copy link
Owner

Hi @MFlisar , a bit late but I added the option autoSize to the Cached annotation. It defaults to true in the version 2.4.1 (available in a few hours on maven) and will use the number of preferences for the cacheSize.

@MFlisar
Copy link
Author

MFlisar commented Jan 19, 2017

Nearly one year ;-). But it's no problem, was using it with a high number and that worked as well. Thanks

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

3 participants