Skip to content
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

DRAFT: Adding JSPI and Asyncify #518

Draft
wants to merge 35 commits into
base: main
Choose a base branch
from
Draft

Conversation

calvinrp
Copy link
Contributor

@calvinrp calvinrp commented Oct 28, 2024

Adds options to jco transpile and jco types:

.addOption(new Option('--async-mode [mode]', 'use async imports and exports').choices(['sync', 'jspi', 'asyncify']).preset('sync'))
.option('--default-async-imports', 'default async component imports from WASI interfaces')
.option('--default-async-exports', 'default async component exports from WASI interfaces')
.option('--async-imports <imports...>', 'async component imports (examples: "wasi:io/[email protected]#poll", "wasi:io/poll#[method]pollable.block")')
.option('--async-exports <exports...>', 'async component exports (examples: "wasi:cli/run@#run", "handle")')

Which treats the specified imports and exports as async using either JSPI or Asyncify.

JSPI is behind a feature flag in Chrome, Deno and NodeJS. Can run via:

DENO_V8_FLAGS="--experimental-wasm-jspi" deno
node --experimental-wasm-jspi

From my understanding, Asyncify does not support reentrancy (so it throws an error) but JSPI does.

@calvinrp calvinrp marked this pull request as draft October 28, 2024 22:42
Copy link
Collaborator

@guybedford guybedford left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very exciting to see this, thank you.

Left some initial comments.

Comment on lines +71 to +91
record async-imports-exports {
imports: list<string>,
exports: list<string>,
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since components can contain multiple sub-components, this is currently joining imports and exports across all sub-components.

If we only consider the outer component import and exports, then we should restrict to that, but we can't do that either since internal components must be async if the outer component is!

Therefore I think we should probably restrict this to outer component imports only

And we should make exports all async as soon as we go into asyncify mode. This also will then help avoid the easy footgun of inadvertantly not making an export async, and then finding it calls an async import down the line which would trap.

crates/js-component-bindgen/src/function_bindgen.rs Outdated Show resolved Hide resolved
crates/js-component-bindgen/src/function_bindgen.rs Outdated Show resolved Hide resolved
crates/js-component-bindgen/src/transpile_bindgen.rs Outdated Show resolved Hide resolved
crates/js-component-bindgen/src/transpile_bindgen.rs Outdated Show resolved Hide resolved
Comment on lines +17 to +26
const DEFAULT_ASYNC_IMPORTS = [
"wasi:io/poll#poll",
"wasi:io/poll#[method]pollable.block",
"wasi:io/streams#[method]input-stream.blocking-read",
"wasi:io/streams#[method]input-stream.blocking-skip",
"wasi:io/streams#[method]output-stream.blocking-flush",
"wasi:io/streams#[method]output-stream.blocking-write-and-flush",
"wasi:io/streams#[method]output-stream.blocking-write-zeroes-and-flush",
"wasi:io/streams#[method]output-stream.blocking-splice",
];
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It could be nice to automatically include a preview2 implementation designed for async now, and run the preview2 tests against that.

One issue here is how to get the async imports, but an easy option might be to just use a convention like:

import ... from '@bytecodealliance/preview2-shims/async/io'`;

This would be amazing to explore further...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, I have a working draft of WASI P2 shims that I have been testing against. But was thinking of breaking into a separate PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants