Open mfbx9da4 opened 2 years ago
Yep, that's definitely possible!
I'm not sure if I'd be happy adding an async method for every sync method though, because the API will look a bit messy then. Right now it's very simple and minimal. 😅 Maybe we can find a way around that
Either way, it's a little bit of work, not sure how much exactly but we need to set up a Promise resolver that runs on a separate Thread, custom batched methods, etc etc
API wise maybe the counterpart to all the standard methods eg. mmkv.getString()
and mmkv.getNumber()
could be mirrored under the property async ie mmkv.async.getString()
, mmkv.async.getNumber()
, mmkv.async.getMany()
?
Do you like that? @mrousavy
Another hacky thing you could do is have mmkv.getString.async()
, I've seen Zustand do that but I kind of hate it and it won't help with batch methods.
Personally I think the API mmkv.getStringAsync()
and mmkv.getNumberAsync()
are the most natural as they will be the easiest to find for typescript users and I think I've seen some NodeJS APIs use that pattern.
Just one question around thread safety, say for example we implement the method mmkv.setManyAsync(items: Array<[string, string | number | boolean])
. Will it actually be thread safe to create a thread and then inside the thread do convertJSIStringToNSString(argument.getNumber())
, argument.getBool()
, argument.getNumber()
or will that have to be done outside the thread?
Arg parsing will need to happen outside, but that's fast anyways.
I'm thinking of a potential solution for setting.
I can implement one additional method on the MMKV instance called batch
.
const storage = new MMKV()
await storage.batch((s) => {
s.set('hello', 'world')
s.set('test', 'another')
// ...
})
Inside you will use the s
instance (same MMKV
instance, but a Proxy), which will schedule the methods instead of actually running them.
The only problem I see with this now is that you cannot use getters (s.getString
, s.getNumber
, s.getBoolean
) because those would have to be synchronous.
A "solution" for that would be to make those funcs async, but that's a bit complicated again.
Perhaps batch would have to return an array of items, does that solve it?
const storage = new MMKV()
const [foo] = await storage.batch((s) => [
s.getString('foo'),
s.set('hello', 'world'),
s.set('test', 'another')
])
Also could you not make storage to be context aware, then you could have this API:
const storage = new MMKV()
const [foo] = await storage.batch(() => [
storage.getString('foo'),
storage.set('hello', 'world'),
storage.set('test', 'another')
])
Provided you enforce that batch is always a synchronous function.
BTW, I love the batch() idea
Perhaps batch would have to return an array of items
That's a good idea, but then you'd be very limited in terms of doing stuff contextually (e.g. only set if the value in storage is ....), but yea that would probably work.
Provided you enforce that batch is always a synchronous function.
Wdym? The whole idea is to have batch asynchronous and awaitable
Wdym? The whole idea is to have batch asynchronous and awaitable
Sorry I mean the batch implementation would enforce that the batch argument callback is synchronous, this is so that the batchFn doesn't need to be injected with a proxy.
class Storage {
// ...
batch(fn) {
ensureSynchronous(fn)
this._batchingEnabled = true
try {
const items = fn().filter(x => x)
items.every(x => ensureIsBatchItem(x))
return await this._executeBatch(items)
} finally {
this._batchingEnabled = false
}
}
}
but then you'd be very limited in terms of doing stuff contextually (e.g. only set if the value in storage is ....)
What use case might be limited?
eg. this (but on a larger scale with tons of items like arrays):
const storage = new MMKV()
await storage.batch((s) => {
const x = s.getBoolean('bla')
if (x === true) {
s.set('hello', 'world')
} else {
s.set('test', 'another')
}
// ...
})
Hmm yeah it's true that wouldn't work and having a callback does give the impression that it should be possible. One saving grace is that typescript users will get the following error:
const storage = new MMKV()
await storage.batch((s) => {
const x = s.getBoolean('bla')
// ❌ This condition will always return 'false' since the types 'MMKVBatchItem' and 'boolean' have no overlap
if (x === true) {
s.set('hello', 'world')
} else {
s.set('test', 'another')
}
// ...
})
I don't see how you could support the above API, unless... you implemented a similar strategy to react-native-multithreading and all batchFn
s were marked automatically with a "worklet" like tag? While it's probably overkill for MMKV, it's fun to dream.
BTW one thing which is way better about .batch
vs storage.async.{{method}}
is that there will be less back and forth between Microtasks Queue, C++, JSI and JS and you will only pay the cost once per batch. Whereas, with my original API you would expect to pay the cost three times here:
await Promise.all([
storage.async.getString('foo'),
storage.async.getString('bar'),
storage.async.getString('baz')
])
Although, thinking about it more, even with the .async
approach, in theory you could batch any calls within the same microtask 🤔. Still though, explicit batching is probably preferable.
Also maybe just twitter poll the options?
+1 For async, as it will make larger buffer storage play nicely :)
In the meantime, is there any way to wrap the existing methods so they don't block the main thread ?
We have a project where mmkv overgrown it and currently blocks often main thread
Yea I think the async batch methods are cool, there are some things we need to think about though. I also need a lot of free time to work on this, so if you want to see this happening consider sponsoring me / emailing me about those efforts
+1 For async, as it will make larger buffer storage play nicely :)
In the meantime, is there any way to wrap the existing methods so they don't block the main thread ?
Not really, if the underlying implementation is synchronous. The best you could do would be to defer execution to the next tick as a microtask, using promises, requestAnimationFrame, timeout, etc. E.g.
const resolvedPromise = Promise.resolve()
const setAsync = (k: string, v: string) => resolvedPromise.then(() => storage.set(k, v))
setAsync('foo', 'bar')
This would defer to the next tick, running before any timeouts, but it would still block. Async in this way is only useful if your underlying implementation eventually uses threads and doesn't block the main thread. Otherwise, you're just blocking later instead of now.
I was wondering if it would be possible to delegate saving to MMKV to Worklet, so it doesn't block the main thread? I was trying to implement that but I'm facing issues with executing mmkv instance methods within the worklet. And I'm actually worried that the reason is that I don't really understand the concept of worklets properly 😅
This shouldn't be implemented on worklets, because data sharing with other thread is a big overhead
that makes sense, thanks for explanation @XantreGodlike!
Is this feature planned for in the future?
Is this feature planned for in the future?
Thank you very much, we are looking forward to it!
While mmkv is super fast, and that's awesome 😎 it still blocks the main JS thread and for situations where you have to get a bunch of keys in a loop of some kind, that can add up. It would be great if mmkv could provide a threaded async version where
jsCallInvoker
Do you think that would be possible?