Shared memory support#9507
Conversation
github-actions
Bot
added
fuzzing
Subscribe to Label Actioncc @fitzgen DetailsThis issue or pull request has been labeled: "fuzzing", "wasmtime:api", "wasmtime:c-api"Thus the following users have been cc'd because of the following labels:
To subscribe or unsubscribe from this label, edit the |
| MemoryType::new64(minimum, maximum) | ||
| } else { | ||
| MemoryType::new( | ||
| Box::new(if shared { |
There was a problem hiding this comment.
Mind panicking here for now with unimplemented!() if both shared and memory64 are set? (we need to add a constructor for that but haven't gotten around to it yet)
There was a problem hiding this comment.
Actually I take this back, this may want to use MemoryTypeBuilder to support shared 64-bit memories
|
Suggestions applied! |
|
Thanks! I think we'll still want to remove the |
|
Done! |
alexcrichton
left a comment
alexcrichton
left a comment
There was a problem hiding this comment.
Looking good! With MemoryTypeBuilder for the implementation I think this will be good to go 👍
| /// Construct a dummy shared memory for the given memory type. | ||
| pub fn dummy_shared_memory(engine: &Engine, ty: MemoryType) -> Result<SharedMemory> { | ||
| SharedMemory::new(engine, ty) | ||
| } |
There was a problem hiding this comment.
Is this still needed? (or should it perhaps be integrated elsewhere?)
There was a problem hiding this comment.
Nah, I just removed it
* moving dummy shared memory
|
Oh, nice, the MemoryTypeBuilder made the code way more readable :) |
alexcrichton
left a comment
alexcrichton
left a comment
There was a problem hiding this comment.
There's a minor issue with clang-format in CI, but otherwise I think this is good to go 👍
|
Oh, no problem. I can run clang-format and see a green CI :) |
|
I think there might be something else with clang-format in CI? If you'd like also feel free to mark this as "ready for review" so it can be approved+merged once CI passes. |
|
Yeah, might be the version of clang format usedd in the CI (15?) has different default options than the one I use locally. |
|
I dunno what's going on with clang-format myself, I've attempted to resolve the latest issue with a new commit |
2 participants
I was trying to add Shared Memory support in the wasmtime-py library, but I found out that C-API support here was not finished, so this is my bold attempt to implement something that works.
As far as a I read, threads on WASI are still in discussion and Shared Memory is part of the proposal. In the other hand there's some basic support for Shared memory already present in the Rust API interface, so I guess it's fine to try gluing the C-API with what is supported today, and make wasmtime-py works with Shared Memory (?).