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

Port wasmtime-fiber to no_std and allow async feature in no_std Wasmtime. #9689

Merged
merged 1 commit into from
Dec 4, 2024

Conversation

cfallin
Copy link
Member

@cfallin cfallin commented Nov 27, 2024

This PR allows a no_std Wasmtime build to be configured with the async feature. (Previously, a minimal no_std configuration could only run with sync entry points, without suspending of stacks.)

The main hurdle to this support was the wasmtime-fiber crate. Fortunately, the "unix" variant of fibers was almost entirely portable to a no_std environment, owing to the fact that it implements stack-switching manually in assembly itself. I moved the per-ISA implementations to a shared submodule and built the nostd platform backend for wasmtime-fiber with a stripped-down version of the unix backend.

The nostd backend does not support mmap'd stacks, does not support custom stack allocators, and does not propagate panics.

I've updated the min-platform example to ensure this builds but have not yet actually tested it (I am in the middle of a larger porting effort); hence, a draft PR for initial feedback.

@cfallin
Copy link
Member Author

cfallin commented Nov 27, 2024

cc @alexcrichton -- I think you're on vacation now but I'd be very curious what you think of this in general!

@cfallin cfallin changed the title Port wasmtime-fiber to no_std and allow async feature in no_std Port wasmtime-fiber to no_std and allow async feature in no_std Wasmtime. Nov 27, 2024
@github-actions github-actions bot added the wasmtime:api Related to the API of the `wasmtime` crate itself label Nov 27, 2024
crates/fiber/src/lib.rs Outdated Show resolved Hide resolved
@cfallin cfallin force-pushed the async-nostd branch 3 times, most recently from 2cfc402 to dc4fe18 Compare November 27, 2024 17:52
Copy link
Member

@alexcrichton alexcrichton left a comment

Choose a reason for hiding this comment

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

This all looks quite reasonable to me, happy to see this merged 👍.

Could you also add a cargo test -p wasmtime-fiber --no-default-features in CI? I generally try to make sure that if there's nontrivial code added with no_std/etc (as is the case for the nostd.rs file here) that we have some at least smoke testing of what's going on. I think that this should be testable when the std feature is disabled? (might have to conditionally enable/disable some panicking tests, and it's ok to use std in tests just want to be sure to test nostd.rs somehow)

.github/workflows/main.yml Outdated Show resolved Hide resolved
crates/fiber/Cargo.toml Outdated Show resolved Hide resolved
crates/wasmtime/Cargo.toml Outdated Show resolved Hide resolved
examples/min-platform/embedding/Cargo.toml Outdated Show resolved Hide resolved
crates/fiber/src/lib.rs Outdated Show resolved Hide resolved
crates/fiber/src/lib.rs Outdated Show resolved Hide resolved
crates/fiber/src/lib.rs Outdated Show resolved Hide resolved
crates/fiber/src/lib.rs Outdated Show resolved Hide resolved
@cfallin cfallin marked this pull request as ready for review December 4, 2024 00:26
@cfallin cfallin requested review from a team as code owners December 4, 2024 00:26
@cfallin cfallin requested review from alexcrichton and removed request for a team December 4, 2024 00:26
@cfallin
Copy link
Member Author

cfallin commented Dec 4, 2024

Updated and out of draft mode now; thanks!

Copy link
Member

@alexcrichton alexcrichton left a comment

Choose a reason for hiding this comment

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

Is it possible to add a `cargo test -p wasmtime-fiber --no-default-features to CI as well?

@cfallin cfallin force-pushed the async-nostd branch 2 times, most recently from 682e9b1 to 1ff8b54 Compare December 4, 2024 05:50
@cfallin
Copy link
Member Author

cfallin commented Dec 4, 2024

Sure thing, done!

@cfallin cfallin enabled auto-merge December 4, 2024 05:52
@cfallin cfallin added this pull request to the merge queue Dec 4, 2024
github-merge-queue bot pushed a commit that referenced this pull request Dec 4, 2024
… Wasmtime. (#9689)

This PR allows a `no_std` Wasmtime build to be configured with the
`async` feature. (Previously, a minimal `no_std` configuration could
only run with sync entry points, without suspending of stacks.)

The main hurdle to this support was the `wasmtime-fiber` crate.
Fortunately, the "unix" variant of fibers was almost entirely portable
to a `no_std` environment, owing to the fact that it implements
stack-switching manually in assembly itself. I moved the per-ISA
implementations to a shared submodule and built the nostd platform
backend for `wasmtime-fiber` with a stripped-down version of the unix
backend.

The nostd backend does not support mmap'd stacks, does not support
custom stack allocators, and does not propagate panics.

I've updated the `min-platform` example to ensure this builds but have
not yet actually tested it (I am in the middle of a larger porting
effort); hence, a draft PR for initial feedback.
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Dec 4, 2024
@cfallin cfallin enabled auto-merge December 4, 2024 06:02
@cfallin cfallin added this pull request to the merge queue Dec 4, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Dec 4, 2024
@cfallin cfallin added this pull request to the merge queue Dec 4, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Dec 4, 2024
@cfallin
Copy link
Member Author

cfallin commented Dec 4, 2024

The CI failures are fairly perplexing -- the only failing job I see is "report failure on cancelation", and a bunch of other jobs are canceled midway through their runs. @alexcrichton any ideas?

@alexcrichton
Copy link
Member

This happened on another PR the other night and I couldn't figure out what was happening. I then re-queued in the morning and it merged so I assumed it was a transient failure in CI infrastructure... Let's see if that still holds true?

@alexcrichton alexcrichton added this pull request to the merge queue Dec 4, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Dec 4, 2024
@alexcrichton
Copy link
Member

I've pushed a commit to run full CI on this PR where the PR run can't cancel itself, so let's see what happens...

@alexcrichton
Copy link
Member

Bleh ok so this is a known issue (at least to me) on github actions. For the merge queue there's no way to cancel a failed job so our main.yml self-cancels whenever a job fails. This works fine on all platforms except Windows. On Windows self-cancelling sometimes gives you a big red X on the job and sometimes gives you the icon of "this was cancelled by someone else". Basically there seems to be a race of the self-cancel and the job actually failing.

In any case it's the Windows build. Just some minor things to update.

@cfallin cfallin force-pushed the async-nostd branch 2 times, most recently from b00174c to 65d9ef8 Compare December 4, 2024 18:43
@cfallin
Copy link
Member Author

cfallin commented Dec 4, 2024

Ah, that was entirely unintuitive, GitHub CI user-interface! Thanks for working that out.

… Wasmtime.

This PR allows a `no_std` Wasmtime build to be configured with the
`async` feature. (Previously, a minimal `no_std` configuration could
only run with sync entry points, without suspending of stacks.)

The main hurdle to this support was the `wasmtime-fiber` crate.
Fortunately, the "unix" variant of fibers was almost entirely portable
to a `no_std` environment, owing to the fact that it implements
stack-switching manually in assembly itself. I moved the per-ISA
implementations to a shared submodule and built the nostd platform
backend for `wasmtime-fiber` with a stripped-down version of the unix
backend.

The nostd backend does not support mmap'd stacks, does not support
custom stack allocators, and does not propagate panics.

prtest:full
@cfallin cfallin added this pull request to the merge queue Dec 4, 2024
Merged via the queue into bytecodealliance:main with commit abcd6ac Dec 4, 2024
130 checks passed
@cfallin cfallin deleted the async-nostd branch December 4, 2024 19:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
wasmtime:api Related to the API of the `wasmtime` crate itself
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants