Back with Keyv

[lukechilds]

Redirected here from mem sindresorhus/memoize#15 (comment)

Are you interested in backing this with Keyv?

lukechilds/keyv

It would work in memory like it does currently but give users the option to use a persistent store like Redis/Mongo/Postgres/etc and share memoized results between servers/restarts.

[sindresorhus]

I like the idea. It would make this module very powerful. But I also want to optimize for the common case, which is just using the default Map caching, and there are also size concerns as this module is often used in the browser. How about not depending on keyv directly, but instead tell the user to install it if they want to use the cache option?

[SamVerschueren]

I'm interested in this as well. What I want is to persist my cache on S3 so my cache can be easily shared between AWS lambda instances.

I will have a look if we could easily add support for async storage providers like Keyv or some other custom providers. Would make it really nice!

Will probably do it this week as I really need it :).

[lukechilds]

@SamVerschueren There is a community adapter for Dynamo DB: https://github.com/e0ipso/keyv-dynamodb

Should be faster than S3 and be accessible from AWS Lambda.

I haven't had the time to look into it too much but might be useful for you.

[ClaudiuCeia]

So, I tried building a redis cache adaptor for mem, then found the other thread which redirected here. I just switched mem for p-memoize but the cache object signatures are still synchronous even for this one.

Maybe I'm missing something but it seems to me like this isn't a huge task, it's just that the Cache object being passed in the options doesn't get awaited on any of the IO methods (get, set, has, etc.) That also probably means you can't really use mem as a dependency here since async goes all the way up, no?

declare namespace pMemoize {
    interface Cache<K = string, V = any> {
        get(key: K): V;
        set(key: K, value: V): void;
        has(key: K): boolean;
        delete(key: K): void;
        clear?(): void;
    }
}

I did a quick push so I can show you what I'm doing: https://github.com/ClaudiuCeia/p-memoize-redis/tree/master/src.

It's TypeScript but I can transpile to JS and publish on npm so it can be used just as a drop-in for the options param.

[sindresorhus]

Maybe I'm missing something but it seems to me like this isn't a huge task, it's just that the Cache object being passed in the options doesn't get awaited on any of the IO methods (get, set, has, etc.) That also probably means you can't really use mem as a dependency here since async goes all the way up, no?

Correct. Luckily mem is not massive, so we can just copy and adapt the code from there.

[fregante]

What's the action on this issue? This?

• copy mem to p-memoize ``
• make every call return a promise

[sindresorhus]

@bfred-it Yes

[fregante]

I'm not entirely sure what the problem currently is. The supplied map has to return a V value but mem doesn't really care what that value is, right? V is returned as is an the user can await it if it's a Promise

[sindresorhus]

@bfred-it I haven't looked into it. You might be right.

[fregante]

I tested this; for this to work, we have to await any has and get calls in the module, for example: https://github.com/sindresorhus/mem/blob/ce7f3b7135e32d80877dedf0f0b7bd3e14d0ad91/index.js#L38-L40

if (cache.has(key)) { // if (promise)
	return maxAge ? cache.get(key).data : cache.get(key); // promise.data === undefined
}

[kmohrf]

I just stumbled upon this, when I wanted to use mem with an async cache backend based on idb-keyval. It makes sense to implement this in p-memoize instead of mem, but I think it would make a very powerful addition, since we’d be able to use basically any browser-based storage option. Right now I have to fallback to sync localStorage which really is a pity. So: all the thumbs up for supporting async caches! :)

[fregante]

Now that sindresorhus/memoize#43 is in and mem has been simplified, this can finally be done.

PR welcome on p-memoize to copy mem in its entirety (docs and tests), plus:

• await the .has and .get calls as suggested in Back with Keyv #3 (comment)
• update the docs and tests to include async examples

[Zikoel]

Is this feature working? From the code is not completely clear for me... I see the cache option is passed directly to the underlying mem lib so seems that is not possible to operate with an async cache right now!

[Kikobeats]

@Zikoel you're right, at this moment p-memoize is not compatible with async cache storage since mem is using sync methods:

https://github.com/sindresorhus/mem/blob/main/index.ts#L117

If you need to memoize working with keyv, I recommend using memoized-keyv before this is resolved.

[papb]

@Kikobeats Don't link to specific lines like that, if a new commit changes anything your link will become confusing. Get a permalink instead, like this: https://github.com/sindresorhus/mem/blob/a42d7205cc7939e1738eda2a785fe1ec5bda2760/index.ts#L117

[Richienb]

Since p-memoize is asynchronous, what should it do when it is called twice in a row? Should it treat both as a single call or if not, should the functions just race to be the first to cache a value?

[Richienb]

@sindresorhus How do you think this should be handled?

[sindresorhus]

I think it should coalesce, so the second call just receives the same promise as the first call.

[Kikobeats]

just in case you are interested, we forked keyv to keyv.js.org to keep it running and maintained since the original project is stuck 👍

[JaneJeon]

Hmm, so for now, we just stick to synchronous caches like quick-lru only?

[Richienb]

Hmm, so for now, we just stick to synchronous caches like quick-lru only?

Yes

[Richienb]

Plan:

Copy mem except for the use of 2 maps: a promise cache and the (already-existing) promise return value cache. When executing the function, return a cached promise or the cached value after passing to Promise.resolve (if not already in an async/await function) if available. Otherwise, run the promise and add it to the promise cache. When the promise completes and it resolves add the return value to the return value cache and remove the cached promise if caching rejected promises is disabled before returning the value. This will allow another execution to pick the value back up again.

This system is required because it is not possible to cache the Promise object in a database and as such, these things can not be cached between executions:

• Custom properties like .cancel if they exist on Promises.
• Rejected promises that throw custom errors (for normal errors we could just use serialize-error but that is not one-size-fits-all).
• Symbols.

There are 3 solutions I can think of:

1. We could stringify the Promise object and make the functions no-ops but the Symbols won't persist and it might be consuming too much database space with things only 1% of users will actually need.
2. Perhaps a value on the resolved promise could be set like isCachedValue to show that the value was fetched from the database and not from the local promise cache.
3. Otherwise, we can just document this downfall. I'll go with this solution for now.

For users that still want the previous behaviour, should we add a seperate function for that or just remove this functionality?

Also, what should the promise cache option be called, promiseCache?

[Richienb]

If the following events happen, what should p-memoize do?

1. Function is executed on instance A
2. Function is executed on instance B
3. Function on instance A resolves
4. A second function is executed on instance B

Should the second function return the cached promise or the cached value? They can't be raced against each other because the promise cache is synchronous. Right now, the promise cache will be checked first then the value cache. Also keep in mind that promise rejections can't entirely be cached across instances.

[Richienb]

Because different cache providers have different ways in configuring cache expiry, setting maxAge is not ideal since that property name is only intended for use in map-age-cleaner.

For now, I've removed that option since the cache should be able to manage it's own value expiry however way it is supposed to be done on the target database. If not, then mem might be a better option for consumers.

[Richienb]

• await the .has and .get calls

Other calls may run into a local race condition where once .clear is called in the .pMemoizeClear() function, the cache is not immediately cleared. TypeScript doesn't really like all the methods being async functions because of some sort of conflict but I might be able to sort that out if need be.

Also, should .pMemoizeClear() also clear the promise cache? If so, after the promise cache is cleared should any still running instance of the promise still cache its resolved value or be added back to the promise cache when it resolves? For now, the promise cache will be cleared and newly resolved values will still be added.