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

Add multi-memory example #35

Merged
merged 7 commits into from
Apr 15, 2024
Merged

Conversation

hamishwillee
Copy link
Collaborator

This adds an example of how to use multiple memories in Web Assembly.

  • It shows how to create two memories in JavaScript and import to the WASM, and one memory in WASM and export to the JavaScript.
  • It updates each with different text that gets printed to screen (uses screen based logging rather than console logging, which is much nicer)
  • It shows that the default memory is the 0 index one.

Still working on docs

This is part of work for mdn/content#32777

@hamishwillee
Copy link
Collaborator Author

@bsmth Do you have reviewer rights here?

@bsmth
Copy link
Member

bsmth commented Apr 9, 2024

@bsmth Do you have reviewer rights here?

I do, yes. I can take a look - adding myself as a reviewer

@bsmth bsmth self-requested a review April 9, 2024 12:15
@hamishwillee
Copy link
Collaborator Author

Thanks @bsmth !

@hamishwillee
Copy link
Collaborator Author

FYI, would be good if this could be merged somewhat soon. The release is this week.

@bsmth
Copy link
Member

bsmth commented Apr 15, 2024

Thanks, Hamish. To get this running I did:

# Get wabt tools (https://github.com/WebAssembly/wabt)
brew install wabt

wat2wasm --enable-multi-memory multi-memory.wat -o multi-memory.wasm

Should we capture that somewhere? Can be on the content side

@bsmth
Copy link
Member

bsmth commented Apr 15, 2024

Back over to you, @hamishwillee - do you want to check the comments I've left? I think we can merge afterwards 🚢

Co-authored-by: Brian Thomas Smith <brian@smith.berlin>
Copy link
Member

@bsmth bsmth left a comment

Choose a reason for hiding this comment

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

LGTM pending the comment about overrunning the memory, the code comment looks like a good way to flag what's happening

@bsmth bsmth merged commit fb0118a into mdn:main Apr 15, 2024
@hamishwillee hamishwillee deleted the multi-memory-example branch April 15, 2024 23:28
@hamishwillee
Copy link
Collaborator Author

@bsmth Thanks for merging (and happy with your changes)

Should we capture that somewhere? Can be on the content side

I am not sure. The "right" think to do would be to allow support for building WASM in live examples. However that's a bit of a step because you'd need to be able to dynamically generate wasm from wat. It is actually not possible for this example because the tools that are used by interactive examples don't support the multimemory (which is what we'd used).

I don't think we'd do it in content.

The best compromise way to do it would be to add "Running/Setup" into this project's README. Thoughts?

@bsmth
Copy link
Member

bsmth commented Apr 16, 2024

Thanks, Hamish!

I wasn't sure if we covered how to do this in a generic way in mdn/content docs, but I see we have this: https://developer.mozilla.org/en-US/docs/WebAssembly/Text_format_to_Wasm#converting_the_text_.wat_into_a_binary_.wasm_file which is exactly what I was looking for, so problem solved. We could link to that from the README, I think that's a good help.

On a related note, I see there's some work ongoing for wat support:

@hamishwillee
Copy link
Collaborator Author

Thanks @bsmth - that's cool, particularly the bob update. Though it is hard to tell from https://crates.io/crates/wasm-pack or the bob updates whether the features will "all" be supported (or even which ones are).

Fixed up readme here #36

@bsmth
Copy link
Member

bsmth commented Apr 19, 2024

whether the features will "all" be supported (or even which ones are).

Good question, Florian / Vadim would have better context on it, it looks promising so far!

hamishwillee added a commit that referenced this pull request Apr 21, 2024
@bsmth As discussed in
#35 (comment)

This adds links to WABT, the docs on MDN, and to the table for supported
features.

---------

Co-authored-by: Brian Thomas Smith <brian@smith.berlin>
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