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

memory.discard in a separate proposal? #7

Open
llenotre opened this issue Aug 16, 2023 · 7 comments
Open

memory.discard in a separate proposal? #7

llenotre opened this issue Aug 16, 2023 · 7 comments

Comments

@llenotre
Copy link

Hi!

I think it would be better to put the memory.discard instruction into a separate proposal.

Grouping this instruction with other features is not strictly necessary and slows down the work on it. Current open questions only concern other features, and this proposal doesn't seem to be very active at the moment.
The instruction even already has been implemented on SpiderMonkey.

I believe that having this instruction standardized quicker would be beneficial as there is currently no way to explicitly restore memory back to the OS, which is a major usability issue for mobile usecases.

@bvisness
Copy link
Collaborator

It might make sense to split it into its own proposal if others agree. However, I would prefer to consider the memory story of WebAssembly more broadly, and given the lack of activity across this entire proposal, I wouldn't feel comfortable splitting it out without more discussion from multiple parties.

It's very possible that landing memory.discard in core WebAssembly would make life very difficult when implementing, say, copy-on-write views of JS typed arrays. What happens when a memory.discard spans plain old linear memory and a typed array view? Who knows? Do we want a world where we have to handle this?

A personal concern for me is whether memory APIs are best implemented in core WebAssembly or on the host. Obviously my prototype of memory.discard is currently a core WebAssembly instruction, but in retrospect I think that may have been a poor choice. As WebAssembly implementations proliferate outside the browser, it becomes increasingly important to design carefully around that boundary.

TL;DR until anyone can devote active attention to this, I'd rather not split it out.

@llenotre
Copy link
Author

It's very possible that landing memory.discard in core WebAssembly would make life very difficult when implementing, say, copy-on-write views of JS typed arrays. What happens when a memory.discard spans plain old linear memory and a typed array view? Who knows? Do we want a world where we have to handle this?

Is this issue also present for the memory.fill instruction?
Let's say we fill a region of memory with zeros using memory.fill. What is the difference with using the memory.discard instruction on that exact same region of memory?

@penzn
Copy link
Contributor

penzn commented Aug 17, 2023

Discard makes the address range "disappear" - while no indices need to change the underlying memory gets deallocated (this is realistically achieved by memory mapping in the runtime).

It's very possible that landing memory.discard in core WebAssembly would make life very difficult when implementing, say, copy-on-write views of JS typed arrays. What happens when a memory.discard spans plain old linear memory and a typed array view? Who knows? Do we want a world where we have to handle this?

Would this be similar to how array views handle changing memory size (which has some perf implications when many views are open)? I guess array view can also poke into the discarded section.

@llenotre
Copy link
Author

Discard makes the address range "disappear" - while no indices need to change the underlying memory gets deallocated (this is realistically achieved by memory mapping in the runtime).

Yes, but from the point of view of the guest, isn't it the same behaviour? In the implementation of SpiderMonkey, if I understood correctly: if the memory has been discarded and then is accessed again, the OS reallocates it with zero bytes instead.

@penzn
Copy link
Contributor

penzn commented Aug 17, 2023

It changes the footprint - when you deallocate everything on a memory page, it then gets discard'ed (by free() for example) and memory consumption of the tab drops. Without that the only way to reduce the footprint is to delete the instance and start over again.

@bvisness
Copy link
Collaborator

What is the difference with using the memory.discard instruction on that exact same region of memory?

Modifying memory mappings is a very different operation from simply writing to memory. Operating system memory APIs vary quite significantly in what they allow - in particular, while Mac and Linux allow you to mmap on top of previous mappings, Windows has no such concept and prefers you to clear the previous mapping before creating a new one. This gets really thorny when threads are involved. Getting memory.discard to work with shared memories on Windows was quite a hack, and if our hack isn't acceptable, we'll have to significantly rethink our implementation and how it would work with future memory APIs.

Point is, even something as simple as memory.discard might not play nice with a larger story of memory management in wasm.

@dtig
Copy link
Member

dtig commented Sep 19, 2023

I'm not strongly opposed to splitting memory.discard out into its own proposal, but would prefer to keep a logical group of memory instructions/functions if possible for similar reasons mentioned by @bvisness, and emphasizing that making all of the functionality work in a portable manner across threads is challenging. Apologies for the slow progress on this so far, I'm picking this back up again after a hiatus. I'll also add this to the agenda for a future CG meeting for a discussion.

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

No branches or pull requests

4 participants