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

Redesign the API for accessing the contents of a Wasm memory #2646

Merged
merged 3 commits into from
Dec 8, 2021

Conversation

Amanieu
Copy link
Contributor

@Amanieu Amanieu commented Nov 3, 2021

Description

  • Accessing the contents of a memory is now done through the read/read_uninit/write methods of Memory. These functions are thread-safe even when there are concurrent modifications to the memory.
  • The ValueType trait has been reworked to also support zeroing out padding bytes in a type. This is necessary to avoid leaking uninitialized data into Wasm memory.
  • ValueType can now be safely derived with #[derive(ValueType)]. unsafe impl is no longer needed.
  • WasmPtr now dereferences to WasmRef and WasmSlice instead of giving raw access to a Cell.
  • WasmPtr is now generic over the memory size, allowing for 32-bit and 64-bit WasmPtrs. This is done through a MemorySize trait and Memory32/Memory64 types which implement it.
  • WasmRef and WasmSlice types are added. Unlike WasmPtr these types contain a reference to a Memory.
  • MemoryView has been removed, it is superseded by WasmSlice.

Review

  • Add a short description of the change to the CHANGELOG.md file

Copy link
Member

@syrusakbary syrusakbary left a comment

Choose a reason for hiding this comment

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

The PR implementation looks good to me.

As a side comment, I think it might be worth to have read and write operations on a MemoryBuffer (new abstraction) instead than in a Memory.
That way, we will be a bit closer to the js API and we will solve easily the overhead of caching/creating a uint8view.

@Amanieu
Copy link
Contributor Author

Amanieu commented Nov 3, 2021

Could we just create a view when the memory is created and keep that view alive for the whole lifetime of the memory?

@syrusakbary
Copy link
Member

Could we just create a view when the memory is created and keep that view alive for the whole lifetime of the memory?

That sounds good. In the case of JS, if you want to access the memory buffer, you can do memory.buffer (which is always acccessible).

https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/WebAssembly/Memory/Memory

Funnily enough, in JS you need to cast again the buffer into a Uint8Array or similar to really access the data (read/write).
https://developer.mozilla.org/en-US/docs/Web/API/ArrayBufferView

If we could follow a similar API in Rust that would be ideal. Thoughts?

Copy link
Member

@syrusakbary syrusakbary left a comment

Choose a reason for hiding this comment

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

I still think we may want to create a buffer/view abstraction on the future to not operate on read/writes directly on memory, but the current implementation is also good until we decide if we want another abstraction or not.

@Amanieu Amanieu mentioned this pull request Nov 25, 2021
@Amanieu Amanieu merged commit 402737e into wasmer3 Dec 8, 2021
@Amanieu Amanieu deleted the new_mem_api branch December 8, 2021 16:35
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.

2 participants