-
Notifications
You must be signed in to change notification settings - Fork 275
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
Add OPFS support to Wasm bindings #594
base: main
Are you sure you want to change the base?
Conversation
Add basic limbo-opfs-test.html Remove unused browser-its structure - we may bring it back
Change how VFS gets loaded based on feature flags
Hey @elijahmor, looks great! I am totally fine with merging this after a little bit of cleanup. I do want to later on explore if we can leverage Limbo's async core more, but I think following the battle-tested SQLite approach is good. Btw, as follow up, how do we want to package this? Do we need two separate npm packages or is there a way to somehow unify the two? |
Awesome. I'll start cleaning it up tomorrow! As for async my understanding is that Limbo is using its own event loop (I have not dug into that area yet). With async in the browser you have to somehow yield to the js event loop. That was the circle I couldn't square with my current understanding of browser js/wasm/rust. It's something I'm happy to look deeper into, but it'll take me sometime to understand limbos async architecture better. |
@elijahmorg Limbo is designed for an external I/O loop via the |
That's very interesting! I'll take a look at that, but yes I'll keep this approach for now. |
General cleanup of cruft, deleted dead code, moved html files to clean up dir.
"devDependencies": { | ||
"@playwright/test": "^1.40.0", | ||
"@vitest/ui": "^1.0.0", | ||
"happy-dom": "^12.0.0", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's a critical security vulnerability on this version of happy-dom
, could you upgrade it?
FEATURE="web" | ||
fi | ||
|
||
wasm-pack build --no-pack --target $TARGET --no-default-features --features $FEATURE |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could we add wasm-pack
as an npm
dev dependency and call npx wasm-pack
here?
Hey @elijahmorg, any reason to keep this as draft? Even if incomplete, we could merge this to make it easier for others to also contribute on this. |
Lots of cleanup still left to do. Draft PR for adding support for OPFS for WASM build (add support for limbo in browser).
Overall explanation of the architecture: this follows the sqlite wasm architecture for OPFS.
main <> (limbo-worker.js) limbo (VFS - opfs.js) <> opfs-sync-proxy
The main thread loads limbo-worker.js which bootstraps the opfs.js and opfs-sync-proxy.js and then launches the limbo-wasm.js.
At that point it can be used with worker.postmessage and worker.onmessage interactions from the main thread.
The VFS provided by opfs.js provides a sync API by offloading async operations to opfs-sync-proxy.js. This is done through SharedArrayBuffer and Atomic.wait() to make the actual async operations appear synchronous for limbo.
Happy to elaborate more.
I will start cleaning this up after I get a conceptual go ahead.
resolves #531