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

Refactor wasmer interfacing code #3313

Open
3 tasks
sug0 opened this issue May 27, 2024 · 4 comments
Open
3 tasks

Refactor wasmer interfacing code #3313

sug0 opened this issue May 27, 2024 · 4 comments

Comments

@sug0
Copy link
Contributor

sug0 commented May 27, 2024

In #3308, wasmer was bumped to 4.3.1. The migration from 2.x to 4.x came with significant API changes, which were addressed in a less than ideal way. Therefore, we ought to refactor/improve this code. Here are some of the associated tasks:

  • Attempt to remove the unsafe Send and Sync impls on wasm memory. We never share memory across threads, and it is not allocated in thread local storage; the only reason it is marked as Send and Sync is to abide by the requirements of wasmer host functions, which must implement Send and Sync.
  • Reduce boilerplate of wrapping host functions in wrap_tx and wrap_vp modules.
  • Decouple wasmer::Store from WasmMemory. Instead, use the host function wrappers to grab a wasmer::AsStoreMut impl from the wasmer::FunctionEnvMut.
@tzemanovic
Copy link
Member

tzemanovic commented May 28, 2024

I think for the 2nd point (wrap_tx and wrap_vp) we could have something similar to the trait wasmer::HostFunction that would call env.data_mut() for us

@tzemanovic
Copy link
Member

on a second thought, it might be better to use wasmer::FunctionEnvMut in our host_env directly in order to address 3rd point

@sug0
Copy link
Contributor Author

sug0 commented May 28, 2024

on a second thought, it might be better to use wasmer::FunctionEnvMut in our host_env directly in order to address 3rd point

some functionality is mocked to run tests. wasmer is not used in some calls to the host functions, so, unless we mock some kind of VmStore that is used by the VmMemory implementation, it is not advisable to pass FunctionEnvMut to host functions directly

@cwgoes cwgoes added this to the Future breaking upgrade milestone Aug 27, 2024
@sug0
Copy link
Contributor Author

sug0 commented Aug 29, 2024

@cwgoes this shouldn't be consensus breaking, it doesn't change execution semantics, it's more of an internal detail (unless the vm is buggy)

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

No branches or pull requests

3 participants