medikoo / memoizee

Complete memoize/cache solution for JavaScript
ISC License
1.74k stars 61 forks source link

reFetch extension for maxAge functionality #1

Closed medikoo closed 12 years ago

medikoo commented 12 years ago

There are cases where we may want to automatically refresh value after it has expired, not just purge it, this simple functionality extension will address that

puzrin commented 12 years ago

I've proposed a bit different thing. Not autorefresh, but background load after request. For examlpe:

No needs to reload each cache slot, only used ones. Tought, in my use cases, all data will be reloaded anyway. So, no principle difference, wich method will be used.

medikoo commented 12 years ago

@puzrin Thanks for clarification. I get the point, only if data is accessed again and close to expiry date, it's wise to refresh it in background and push forward expiry date (so likely future access won't have to wait).

Indeed if data is requested frequently it's beneficial, and it's more optimized approach than refreshing any cached value when it's expired, very good point, I'll make reFetch to work exactly that way.

The only difference to what you propose is that I would probably do re-fetch on calls made some time before expiry date rather than still serving cached value to those that were slightly later, I think it would be more what you expect. Serving value obtained 10 seconds ago when e.g. maxAge is set to 8 seconds is controversial. Let me know if I still miss something.

puzrin commented 12 years ago

Yeah, -(10-20%) before expire will make the same effect, as +(10-20%) after expire (in real life). No principle difference, but pre-fetching looks more logical, than post-loading.

Probably, this interval could be tuned via options, but i guess, 10% default will be "good for all".

medikoo commented 12 years ago

Thank you! It landed in v0.2.2. I used -33% as default, but it's customizable

puzrin commented 12 years ago

Thanks!

puzrin commented 12 years ago

I looked code a bit. Didn't understood, how do you resolve race conditions? If cache is about to expire, and has multiple async gets, will they cause multiple refresh requests? The same question for start state, when cache is empty.

medikoo commented 12 years ago

@puzrin async logic is separated from maxAge/preFetch logic.

When we're working with asynchronous function, cache hit occurs here: https://github.com/medikoo/memoize/blob/master/lib/ext/async.js#L37

preFetch logic that occurs on cache hit is here: https://github.com/medikoo/memoize/blob/master/lib/ext/max-age.js#L48 Race condition is prevented by L49 and L51. There's no way that value would be reFetched multiple times.

Another call to memoized function at L55 invokes https://github.com/medikoo/memoize/blob/master/lib/ext/max-age.js#L23-27 again, so preFetch configuration is reinitialized then.

puzrin commented 12 years ago

Got it, thanks. I was not sure about preFetchCache intention. Probably, it worth to rename it to preFetchLock or similar, to reduce possible confusion. I fact, that's not cache, but singleton locks.

medikoo commented 12 years ago

Good hint, actually preFetchCache holds either timeout id or race condition lock, but still you're right it deservers a better name. I'll fix that. Thanks!