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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

WASM/WASI threads support #362

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

Conversation

seclerp
Copy link

@seclerp seclerp commented May 31, 2023

What it's all about

This is my in-progress work in adding WASI threads support that is required for running LLVM-based .wasm modules built with threading support.

It also adds 2 env variables to enable threading features:

  • EXTISM_WASM_THREADS for WASM threads
  • EXTISM_WASI_THREADS for WASI threads

As a handy bonus, I've added EXTISM_DEBUG switch for enabling .debug_info().

Status

Currently, I have problems with implementing thread-safe wrappers over raw pointers used for input data (Internal.input) and plugin memory access (Internal.memory) propagation. It's required for spawn thread function coming from wasmtime-wasi-threads.

Also, there are some ownership conflicts with WasiThreadCtx, which wants to have ownership over module and linker for some reason, which is conflicting with local function ownership.

Once the issues above will be resolved, WASI threads should work as expected.

.NET-specific

To not mess with the existing NuGet while testing I'm publishing working changes to the new NuGet Extism.runtime.wasm-threads.win-x64 for local testing.

My thoughts

Maybe only threads-specific stores should have an Internal-like structure with safe pointers to not break everything? 馃
Also, Internal is not quite a good name for such a structure, IMO. I propose something like InstanceContext. Then thread-specific struct will be InstanceThreadContext.


P.S. It's also worth bumping wasmtime dependencies versions, which I've already bumped in terms for the PR. Will revert if you have some sort of compatibility concerns.

@zshipko
Copy link
Contributor

zshipko commented Jun 1, 2023

Thanks - this looks good so far! I'm definitely open to some of your ideas surrounding InstanceContext and InstanceThreadContext.

Would it be easier to use Arc<Mutex<Internal>> (or RwLock) everywhere Internal is currently used instead of wrapping specific fields?

I'll keep digging into this a little more and will be sure to post any updates if I come up with anything.

@seclerp
Copy link
Author

seclerp commented Jun 1, 2023

Thanks!

InstanceContext and InstanceThreadContext

After looking a bit more into the codebase I found that InstanceCtx and InstanceThreadCtx are even more suitable (to be consistent with existing contexts structs naming).

Would it be easier to use Arc<Mutex<Internal>> (or RwLock) everywhere

I think it makes sense. 'll try to do so.

@nilslice nilslice mentioned this pull request Jun 27, 2023
Copy link

@Lizzyrobin4 Lizzyrobin4 left a comment

Choose a reason for hiding this comment

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

Reviewed

@bhelx
Copy link
Contributor

bhelx commented Sep 25, 2023

We will take another look at this for 1.0. In the meantime, @seclerp what was the intention and why did you need threads support?

I seem to recall it had something to do with dotnet on the guest side? if so we have a beta coming up for our dotnet pdk and it should support c# and f# extism/dotnet-pdk#4

@seclerp
Copy link
Author

seclerp commented Sep 25, 2023

Hi @bhelx, it's great to hear that .NET PDK is coming. Yes, when I worked on that draft I tried experimenting with creating a .NET PDK based on the NativeAOT-LLVM project. However, I had no success because of Extism and NativeAOT-LLVM runtime restrictions and differences.

@bhelx
Copy link
Contributor

bhelx commented Sep 25, 2023

@seclerp sorry to hear that. We will keep this feature in mind for 1.0. We have been exploring supporting tokio and async rust for the runtime in #470 so that might make threads an easier feature to enable. But if you have any feedback on our dotnet PDK, like whether or not it will work for you, that would be greatly appreciated.

@mhmd-azeez
Copy link
Collaborator

Hi @bhelx, it's great to hear that .NET PDK is coming. Yes, when I worked on that draft I tried experimenting with creating a .NET PDK based on the NativeAOT-LLVM project. However, I had no success because of Extism and NativeAOT-LLVM runtime restrictions and differences.

@seclerp I am interested in learning more because NativeAOT-LLVM will probably produce much smaller binaries and the resulting plugins will be much faster than the embedded Mono method I went with in extism/dotnet-pdk#4

@kant2002
Copy link

Since @seclerp is busy right now, he summons me. I do not know about specifcally his PDK experiment, but before that we play with NativeAOT-LLVM on bare Extism, so we can try from there. If you have questions interested about NativeAOT-LLVM I would try to answer to best of my knowledge.

@mhmd-azeez
Copy link
Collaborator

Hi @kant2002 I wanted to try out NativeAOT-LLVM, but faced some issues: dotnet/runtimelab#2269 (comment)

Do you have any idea what might be going wrong here?

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.

None yet

6 participants