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

Implement WebAssembly thread-local support for WASIX #3699

Closed
john-sharratt opened this issue Mar 22, 2023 · 4 comments
Closed

Implement WebAssembly thread-local support for WASIX #3699

john-sharratt opened this issue Mar 22, 2023 · 4 comments
Assignees
Labels
🎉 enhancement New feature! 📦 lib-wasi About wasmer-wasi lib-wasix Issues related to the WASIX toolchain. priority-medium Medium priority issue
Milestone

Comments

@john-sharratt
Copy link
Contributor

Goal

Currently wasix uses syscalls for getting and setting local thread variables which adds quite some overhead - instead the TLS area should be used for this thus avoiding this overhead and removing some redundant syscalls.

Impacted syscalls

The syscalls impacted are as follows:

        "thread_local_create" => Function::new_typed_with_env(&mut store, env, thread_local_create::<Memory32>),
        "thread_local_destroy" => Function::new_typed_with_env(&mut store, env, thread_local_destroy),
        "thread_local_set" => Function::new_typed_with_env(&mut store, env, thread_local_set),
        "thread_local_get" => Function::new_typed_with_env(&mut store, env, thread_local_get::<Memory32>),

Rust Standard Library

The rust standard library must be modified in order to use TLS instead of the wasix syscalls - the locations of the thread local implementation is as follows:

https://github.com/wasmerio/rust/blob/wasix/library/std/src/thread/local.rs
https://github.com/wasmerio/rust/blob/wasix/library/std/src/sys/wasix/atomics/thread_local_key.rs

Avoiding memory leaks

Particular attention must be made that when threads exit they do not cause memory leaks, at present this is done by creating a callback that the runtime executes after the thread exits _thread_local_destroy. This method will free any memory used by the local thread variables including invoking any destructors (for unsafe allocations). One could use the pthreads libc implementation to achieve the same thing rather than having this as a runtime responsibility.

wasix-libc

Threading is currently implemented using pthreads in the libc fork here:
https://github.com/wasmerio/wasix-libc

One approach to solving this problem would be to implement rust local thread storage entirely using the pthreads implementation however this is not the only way approach and a fully rust native solution is also possible.

Cargo build

WASIX applications can be compiled using this helpful toolchain fork:
https://github.com/wasmerio/cargo-wasix

Testing

There is a fully working http server that uses local thread storage extensively and thus can be used for functional and non-functional tests - it is located here:
https://github.com/john-sharratt/static-web-server

@john-sharratt john-sharratt added the 🎉 enhancement New feature! label Mar 22, 2023
@Michael-F-Bryan Michael-F-Bryan changed the title Implement WebAssembly local-threads support for WASIX Implement WebAssembly threads-local support for WASIX Mar 28, 2023
@Michael-F-Bryan Michael-F-Bryan changed the title Implement WebAssembly threads-local support for WASIX Implement WebAssembly thread-local support for WASIX Mar 28, 2023
@Michael-F-Bryan Michael-F-Bryan added 📦 lib-wasi About wasmer-wasi priority-medium Medium priority issue lib-wasix Issues related to the WASIX toolchain. labels Mar 28, 2023
@Michael-F-Bryan Michael-F-Bryan added this to the v3.3 milestone Mar 28, 2023
@theduke
Copy link
Contributor

theduke commented Mar 28, 2023

After this is implemented in the WASIX toolchain, support for the custom TLS related syscalls should be removed from the wasix crate.

@ptitSeb ptitSeb modified the milestones: v3.3, v4.0 Apr 20, 2023
@ptitSeb
Copy link
Contributor

ptitSeb commented May 3, 2023

TODO: remove the old syscalls

theduke added a commit that referenced this issue May 18, 2023
thread_local support in wasix was previously implemented via custom
exports provided to modules.

This has been refactored to a more efficient (guest-internal)
implementation in both the wasix libc and the Rust fork.

We kept the exports and related functionality in for a while to allow for
modules to update, but now all the related functionality can safely be
removed.

Closes #3699
@ptitSeb ptitSeb modified the milestones: v4.0, v4.1 May 23, 2023
@ptitSeb
Copy link
Contributor

ptitSeb commented May 23, 2023

Tentative v4.0

@theduke
Copy link
Contributor

theduke commented May 25, 2023

Leftover wasix exports were removed in #3906 .

@theduke theduke closed this as completed May 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🎉 enhancement New feature! 📦 lib-wasi About wasmer-wasi lib-wasix Issues related to the WASIX toolchain. priority-medium Medium priority issue
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants