-
Notifications
You must be signed in to change notification settings - Fork 3.5k
JSPI - Fix idbstore with jspi. #19991
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
Conversation
9235e80 to
5e9299f
Compare
|
I guess this only really matters with ASYNCIFY=2? |
Yeah, asyncify=1 has its own mechanism in handleSleep to start things back up again. I've contemplated unifying asyncify 1 and 2, where we wouldn't use handleSleep or handleAsync in js library functions, but we would instead always expect a promise back from an async function and resume on that promise. Currently, asyncify=1 supports doing either sync or async from an import and if we went to the promise approach, a function would no longer be able to do the sync resume. I suppose if the user really wanted sync they could import a sync only version though. Maybe, @kripken has thoughts on this? |
|
Unifying 1 and 2 sounds good in general. I don't have a feeling for how limiting it would be to not allow sync imports, though. Do we have examples of that in the test suite or elsewhere? |
5e9299f to
ba29be9
Compare
|
I don't think it will really be limiting, more that it could affect performance. Currently with asyncify=1, a JS function that is calling |
|
So far I've been unable to reproduce the test failing locally. I'm going to try with a different version of chrome... |
Could we not return a resolved promise and not unwind, something like that? (I admit I don't totally follow this, though, so I'm probably missing something... is there an example of a particular import that could get slower due to this?) |
215b8aa to
cfaceb2
Compare
Return the promise from handleSleep so wasm is suspended until each idb operation is done. Fixes emscripten-core#19989
cfaceb2 to
26b625b
Compare
Return the promise from handleSleep so wasm is suspended until each idb operation is done.
Fixes #19989