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 a wasmtime::Memory::discard function #9572

Open
Photosounder opened this issue Nov 6, 2024 · 13 comments
Open

Add a wasmtime::Memory::discard function #9572

Photosounder opened this issue Nov 6, 2024 · 13 comments

Comments

@Photosounder
Copy link

Photosounder commented Nov 6, 2024

Edited by @fitzgen: This issue has evolved. For the up-to-date description, see this comment


Original Issue Text

Feature

While memory.discard isn't here yet, we could use a way to shrink Wasm module memory without having to reinitialise it. I propose a Wasmtime function to simply shrink the linear memory.

Context

I have a C library with which I can make native host programs that call Wasm reactor module functions sequentially using Wasmtime and the modules have access to one C function to send commands back to the host. Lots of things can be done this way, as it is I could even shrink memory by reinitialising it, but there could be a better way. In many cases I can even see the exact contents of a module's heap from the host so I can see how much free space there is at the end of the heap.

Benefit

In some situations a Wasm module can temporarily need lots of memory. As it is we're not really able to then release that memory without reinitialising the module. If we had a way to simply directly shrink the linear memory buffer to a given number of pages that would go a long way. In my case the host could shrink it either by user input, its own determination (many of my modules use an allocator that allows the host to clearly identify all the allocations on the module's heap) or the module could even send a message to the host requesting for the memory to be shrank either right away or after wasmtime_func_call() is done.

Implementation

I think a simple function (in terms of the C API, I don't know about the Rust side) like wasmtime_error_t *wasmtime_memory_shrink(wasmtime_context_t *store, const wasmtime_memory_t *memory, uint64_t new_size, uint64_t *prev_size) would do nicely, and it would very simply just shrink the linear memory buffer. We have wasmtime_memory_grow() so why not wasmtime_memory_shrink().

Alternatives

Without this I would wait for the module's function to be done executing, copy its memory up to the new size somewhere else, reinitialise the module so it has a new memory, grow it to the desired size and copy the copy back into the module's new linear memory. My proposal would make things smoother. Even better if it's something effectively just like calling realloc() on a buffer so that shrinking regularly by a page or two would happen in no time.

@fitzgen
Copy link
Member

fitzgen commented Nov 6, 2024

The Wasm language semantics dictate that a linear memory cannot shrink. If Wasmtime added a memory-shrinking API, that would violate the Wasm language semantics. As a project, we care deeply about standards compliance and therefore we consider all deviations from the standard language semantics to be bugs and would never intentionally introduce APIs that enable such deviations. The more that Wasm engines deviate from standards the less portable Wasm becomes in practice, toolchains need to know which engine they are targeting and what that engine's nonstandard behavior is, etc...

So, unfortunately, this proposal as written is not something we would entertain.

That said, there are some workarounds:

  1. First, I would ask: do you actually need to shrink memory? There will be a large virtual memory reservation up front for the Wasm linear memory, but if you aren't actually touching the whole linear memory, then physical pages are never actually mapped to most of those virtual pages.

  2. If you are touching most of the memory as part of performing some "task", and then want to reset the memory for the next "task", then I would suggest throwing away the Instance and its Store and simply creating a new Store and Instance for the next "task". For example, function-as-a-service platforms built on top of Wasmtime create a new Store and Instance for each HTTP request they receive. We've done extensive work to make instantiation fast (the benchmarks report 5 micro second instantiation on my old laptop) and you can use tools like Wizer to pre-initialize the Wasm module itself so that it is immediately ready to handle the "task" once instantiated and doesn't need to do any of its own set up. Note that Wasmtime will use a copy-on-write heap image for the module, so it isn't doing an actual byte-by-byte memory copy on every instantiation. By reinstantiating on every task with the original heap image, you also get temporal safety where a bug (or security vulnerability) that arises while handling one task cannot affect any other subsequent task because the state gets reset.

  3. As a refinement to your described alternative: You can have your Wasm import, rather than define, its memory. Implement a MemoryCreator that uses some region of memory that you mmaped yourself. Reinstantiate for each "task" and in between instantiations, madvise(DONT_NEED) the region of memory beyond the initial heap image to zero it out and allow the OS to reclaim the physical pages while leaving the virtual memory reservation in place. Note that this is effectively equivalent to (2) except that it is less portable, requires a bunch of fiddly virtual memory management on your part, and doesn't benefit from temporal safety because not all of the memory is reset between tasks.

  4. In general, the correct path to take when we need things that the Wasm language semantics does not allow for is to create a new Wasm proposal in the CG (that would be the memory-control proposal in this case) and then prototype it in Wasmtime behind a default-off config knob (no one has stepped up to do this for the memory-control proposal in this case). However, this approach maybe doesn't quite apply in this case because the memory-control proposal is so young and the Wasm CG doesn't have consensus on exactly which parts it wants to standardize yet (particularly with regards to the memory.discard sub-proposal). We make exceptions and implement immature proposals sometimes, but that usually requires that the Wasmtime core maintainers are the champions of the proposal (eg the wide arithmetic proposal or the custom page sizes proposal) or otherwise strongly aligned with the proposal. Neither is the case for the memory.discard sub-proposal of the memory-control proposal.

I'd recommend investigating (1) and (2). Let me know if anything I described wasn't clear or if you have any other questions.

@cfallin
Copy link
Member

cfallin commented Nov 6, 2024

allow the OS to reclaim the physical pages while leaving the virtual memory reservation in place.

This brings to mind an interesting standards-compliant option: we could, in theory, implement memory zeroing operations (memory.fill from the bulk memory opcodes) with madvise, if the specified range covers whole pages and is otherwise large enough for the syscall+TLB maintenance overhead to be worth it. This way, one could have a long-running instance with a large memory but still return physical memory to the OS in a way that has well-defined semantics (memory becomes zeroes).

@fitzgen
Copy link
Member

fitzgen commented Nov 6, 2024

memory.discard is essentially an always-page-aligned (memory.fill (i32.const 0)) and is intended to be implemented that way. We're starting to get into the weeds a bit here, and more into the standards realm than the implementation realm, but part of the problem was that windows lacks an atomic equivalent to madvise(DONT_NEED) (you need two syscalls) which means that it doesn't compose with threads and shared memories. I guess you could force all Wasm to be interruptible, set a flag or whatever to pause all other threads, and then do the pair of syscalls once they are all paused, but that's a bit of a nightmare and a big imposition to force on every single Wasm engine...

@cfallin
Copy link
Member

cfallin commented Nov 6, 2024

Sure -- I guess my general point was that we could in theory implement this optimization today (by recognizing an aligned memory.fill of zeroes in our libcall) without depending on a future Wasm proposal coming through the stages. In other words, my point is that it doesn't need to move into the standards realm :-) Though in the fullness of time, encoding the intent more explicitly is better, I would agree.

(And the beauty of having well-defined semantics means that on Windows we could still do a memset, at the cost of performance... I would personally argue for that over trying to do a pause-the-world thing and the complexity that brings.)

@Photosounder
Copy link
Author

Photosounder commented Nov 6, 2024

The Wasm language semantics dictate that a linear memory cannot shrink. If Wasmtime added a memory-shrinking API, that would violate the Wasm language semantics. As a project, we care deeply about standards compliance and therefore we consider all deviations from the standard language semantics to be bugs and would never intentionally introduce APIs that enable such deviations. The more that Wasm engines deviate from standards the less portable Wasm becomes in practice, toolchains need to know which engine they are targeting and what that engine's nonstandard behavior is, etc...

I understand the logic, however this is a case where seemingly sound logic leads to arguing in favour of the worst option. This is the worst option because it involves dancing around a basic problem in absurd and inefficient ways (recreating the memory but smaller, reinstantiating the module, actually reinitialising the module from the start, not actually shrinking memory but somehow telling different OSes that it's not really needed anymore) when simply we should be able to shrink memories and it shouldn't involve copy operations or anything like that, we should just be able to resize memories externally. If I somehow implemented the suggested function in my library outside of Wasmtime (I don't think I'd know how to do this sadly, I don't suppose I can just realloc() the buffer and change a couple of variables) then me, the people who might want to work with my library and the end users would all be better off for it, so while I understand why you wouldn't want this inside Wasmtime, it would be a good thing if we were able to do this on our own anyway.

The more that Wasm engines deviate from standards the less portable Wasm becomes in practice, toolchains need to know which engine they are targeting and what that engine's nonstandard behavior is, etc...

I mostly disagree with this because what I'm proposing isn't something that affects Wasm modules and how they're made in the sense that it's about the host unilaterally shrinking memory at its own risk. From the point of view of the Wasm module imagine this: your allocator grows the memory with memory.grow, then later it frees some memory which does nothing. Then at some point based on its own determinations the host decides to shrink the memory, and when the allocator uses memory.size again to check if it should (re-)grow the memory it sees the new smaller memory size, and if it's too small for the new buffer it's trying to allocate it uses memory.grow again, all as if the memory was never larger in the first place. I understand why something like this would make people in charge of setting things in stone squeamish, after all a careless module programmer could store the memory size locally without consulting memory.size every time and fail to re-grow memory when needed, which sadly is probably why we won't get a good official way to shrink memory for many years, but this doesn't lead to having to worry about which engine a module will work on while programming the module, you simply shouldn't assume things about memory.size, which unfortunately is hampered in a decidedly not future-proof way by official wording claiming that memories can never shrink (because one day either they will shrink or people will have moved on to something that isn't WebAssembly, so we might as well start making modules while understanding right now that memories will probably one day be able to be shrunk without the module directly asking for it).

If you think about it this way as opposed to thinking about an active memory.discard that the module would have to ask for then from the point of view of making Wasm modules it's all a matter of being aware that memory.size can report a smaller size than before, which of course is contrary to what the specification claims, but the current specification will eventually be wrong about this and people should simply understand the potential non-monotonicity of memory.size right now. I think that an important aspect of my proposal is that the function I propose is to be used by the host at its own risk, because there's no general way to guarantee that the memory won't be shrunk too much, but that's okay because it's incumbent upon the users of this function to make sure they do this safely (in my case I can do this safely when my host looks into the module's allocator's CITAlloc table and it can see where the last buffer ends and this allocator can deal with the memory being shrunk externally). The alternative to having this function to be used at our own risk is to force us to do absurd slow things that aren't any safer (it's not any safer to reinstantiate the module, grow the memory and copy back the contents of the memory because it doesn't solve the problem of the module potentially not knowing that the memory got smaller, the only way around this would be to truly reinitialise everything which is really absurd because of how slow it could be and how you would have to design everything to make sure that works in your case).

That said, there are some workarounds:

  1. First, I would ask: do you actually need to shrink memory? There will be a large virtual memory reservation up front for the Wasm linear memory, but if you aren't actually touching the whole linear memory, then physical pages are never actually mapped to most of those virtual pages.

The whole linear memory will be touched because it only gets enlarged as a result of allocations on the heap, and we can presume that allocations aren't done for nothing and every page will be touched at some point. Another detail is that my allocator writes 0xC5 bytes to the heap when enlarging for the sake of heap cleanliness and readability (my allocator is geared towards visualisation and providing extra information) so the whole thing is touched right away. However once it's freed it doesn't get touched anymore.

  1. If you are touching most of the memory as part of performing some "task", and then want to reset the memory for the next "task", then I would suggest throwing away the Instance and its Store and simply creating a new Store and Instance for the next "task".

While that's an interesting idea it's only applicable in a subset of situations and it changes how everything works, just to avoid shrinking memory. It wouldn't work so well if using lots of memory is part of the initialisation of the module and not what it does after initialisation, unless you reinstantiate after the actual internal initialisation, but once again it all comes down to a roundabout way of shrinking memory. Generally just imagine you have something like MS Paint as a Wasm module, your image is the last buffer in the heap and you just resized your image from 20,000 x 15,000 pixels to 1000 x 750, you wouldn't want to reinstantiate anything, you'd want the same things in memory but you wouldn't need a bunch of what you used before.

  1. As a refinement to your described alternative: You can have your Wasm import, rather than define, its memory. Implement a MemoryCreator that uses some region of memory that you mmaped yourself. Reinstantiate for each "task" and in between instantiations, madvise(DONT_NEED) the region of memory beyond the initial heap image to zero it out and allow the OS to reclaim the physical pages while leaving the virtual memory reservation in place. Note that this is effectively equivalent to (2) except that it is less portable, requires a bunch of fiddly virtual memory management on your part, and doesn't benefit from temporal safety because not all of the memory is reset between tasks.

My library is meant to be maximally portable because to me the whole point of Wasm is that it can run almost anywhere, even where I didn't think it would run, so whatever I do has to work anywhere Wasmtime can work.

@alexcrichton
Copy link
Member

@Photosounder I might encourage you to approach this problem with some more curiosity to different thoughts and opinions about how to solve it. Describing alternatives as "absurd", calling existing modules as "careless", and saying that all existing modules should "simply" do one thing at least gives the impression to me that you're not taking any positions seriously other than your own. Being a maintainer of a project involves balancing concerns all the time, for example neither bug reports nor maintainers are 100% correct all the time, so solutions often require balance and understanding from all parties involved. If you're only interested in solving this problem in one way and are unwilling to engage in design then this is unlikely to see a solution.

@Photosounder
Copy link
Author

@Photosounder I might encourage you to approach this problem with some more curiosity to different thoughts and opinions about how to solve it. Describing alternatives as "absurd", calling existing modules as "careless", and saying that all existing modules should "simply" do one thing at least gives the impression to me that you're not taking any positions seriously other than your own. Being a maintainer of a project involves balancing concerns all the time, for example neither bug reports nor maintainers are 100% correct all the time, so solutions often require balance and understanding from all parties involved. If you're only interested in solving this problem in one way and are unwilling to engage in design then this is unlikely to see a solution.

I apologise for my colourful language, I've always been a bit blunt, mostly by non-French standards (in some countries people only ever say nice-sounding things, so culture shock makes things worse). When I say that something is absurd I don't mean it as an attack, I mean that doing something complicated and slow to avoid doing something simple and fast is absurd because that's what it is.

@cfallin
Copy link
Member

cfallin commented Nov 7, 2024

I mean that doing something complicated and slow to avoid doing something simple and fast is absurd because that's what it is.

Hi @Photosounder -- I can appreciate your frustration here, seeing inefficiency that doesn't seem necessary when considering your use-case, but echoing Alex above, I don't think it's coming off in a particularly productive way. I want to try to help illustrate "the other side" -- what are the constraints we are considering here? (Basically: please assume we're acting rationally, and wouldn't do something "absurd" without reasons forcing us to.)

The main constraint we deal with in this context is standards-compliance. We are working in the context of an umbrella organization that believes in open standards and cross-compatibility. We believe strongly that if we deviate from the standards by enabling other functionality, we could lead the ecosystem down paths that are globally worse than if we held back. This is because of the practical dynamics of how the community, the tools and their users work.

For example: say that we enabled an extension to shrink memory, and it became used and essential by common guest-plugin frameworks. Other Wasm engines might then feel pressure to add this feature too. It might interact in strange ways with other Wasm features that exist now or are under consideration -- multithreading makes this interesting in particular, as alluded to above, because of the possibility of a TOCTOU bug where one thread checks in bounds, another shrinks, then the first accesses (now out of bounds).

The way that such interactions are usually discussed is in the context of the Wasm Community Group (CG), which is part of the standards body that specifies Wasm under W3C. You may think this is absurd or too slow or ... but it's important to give all features their fair discussion to find issues like this. There is a history of late-changing realizations in some features that might not have been caught if things had been baked in and stuck forever too early.

We care about being a "good citizen" here and not forcing anyone's hand by shipping a thing that will become a de-facto standard. If we do that, we circumvent the whole process and reduce the number of options available. This could mean that we never get to ship other features because they would be incompatible / cannot be combined with the semantics we're now stuck with. Or it could mean that we have to deprecate the thing and ship a new version of the thing that is compatible, but support the old version "forever" because it's in wide use, which adds to our maintenance and testing costs. Either one is bad.

For all these reasons, when we have a need where the existing Wasm standard doesn't allow for a use-case to be expressed efficiently, we go to the standards group and propose a fix. @fitzgen actually has a great recent example with his custom-page-sizes proposal: it allows systems with tiny memories to express those tiny memories in standard Wasm, without relying on proprietary extensions, and all its interactions with the rest of Wasm were thoroughly discussed. Now that it's a standard proposal, we can integrate it, and we have.

I'd encourage you to see if you can use one of the workarounds we've discussed above; in parallel, please do feel free to engage in the Wasm CG's processes and voice your support for the "memory control" proposal or others as needed. Thanks!

@Photosounder
Copy link
Author

Photosounder commented Nov 7, 2024

I mean that doing something complicated and slow to avoid doing something simple and fast is absurd because that's what it is.

Hi @Photosounder -- I can appreciate your frustration here, seeing inefficiency that doesn't seem necessary when considering your use-case, but echoing Alex above, I don't think it's coming off in a particularly productive way. I want to try to help illustrate "the other side" -- what are the constraints we are considering here? (Basically: please assume we're acting rationally, and wouldn't do something "absurd" without reasons forcing us to.)

The main constraint we deal with in this context is standards-compliance. We are working in the context of an umbrella organization that believes in open standards and cross-compatibility. We believe strongly that if we deviate from the standards by enabling other functionality, we could lead the ecosystem down paths that are globally worse than if we held back. This is because of the practical dynamics of how the community, the tools and their users work.

For example: say that we enabled an extension to shrink memory, and it became used and essential by common guest-plugin frameworks. Other Wasm engines might then feel pressure to add this feature too. It might interact in strange ways with other Wasm features that exist now or are under consideration -- multithreading makes this interesting in particular, as alluded to above, because of the possibility of a TOCTOU bug where one thread checks in bounds, another shrinks, then the first accesses (now out of bounds).

The way that such interactions are usually discussed is in the context of the Wasm Community Group (CG), which is part of the standards body that specifies Wasm under W3C. You may think this is absurd or too slow or ... but it's important to give all features their fair discussion to find issues like this. There is a history of late-changing realizations in some features that might not have been caught if things had been baked in and stuck forever too early.

We care about being a "good citizen" here and not forcing anyone's hand by shipping a thing that will become a de-facto standard. If we do that, we circumvent the whole process and reduce the number of options available. This could mean that we never get to ship other features because they would be incompatible / cannot be combined with the semantics we're now stuck with. Or it could mean that we have to deprecate the thing and ship a new version of the thing that is compatible, but support the old version "forever" because it's in wide use, which adds to our maintenance and testing costs. Either one is bad.

For all these reasons, when we have a need where the existing Wasm standard doesn't allow for a use-case to be expressed efficiently, we go to the standards group and propose a fix. @fitzgen actually has a great recent example with his custom-page-sizes proposal: it allows systems with tiny memories to express those tiny memories in standard Wasm, without relying on proprietary extensions, and all its interactions with the rest of Wasm were thoroughly discussed. Now that it's a standard proposal, we can integrate it, and we have.

I'd encourage you to see if you can use one of the workarounds we've discussed above; in parallel, please do feel free to engage in the Wasm CG's processes and voice your support for the "memory control" proposal or others as needed. Thanks!

Thank you for explaining your point of view. I can see how you're right to reject my proposal. In a way there's a natural conflict. You're right to not create problems by going against the standard, I'm right to ask for what would benefit me, and the people in charge of the standard are right to be cautious about how they're changing their standard. And as you said for me it's annoying that something technically simple that would solve a problem would be denied in favour of something technically much more twisted. In a way my mistake is to focus on the immediate technical aspect. But also it really feels like something that should be directly solved in some way, even intuitively it feels wrong that something can only grow but never shrink.

Which brings me to an idea I just had. Since you guys can't add a function that contradicts the standard, maybe you can add a function that doesn't contradict the standard, yet solves the problem on demand. I've been focusing on actually shrinking the memory because from a technical point of view this is simple and trivial to implement in Wasmtime, but since we can't actually do this for all the reasons outlined we could have a function much like what I suggested but instead of actually shrinking the memory it could mark memory pages as committed (please note that I don't know much about the topic of directly dealing with pages in that kind of way, so not only do I not fully appreciate the technical feasibility of this but also that's why I'd prefer if it was done by Wasmtime so I wouldn't have to write multi-platform code to do this myself) which in practical terms would (I suppose) adequately deal with large amounts of memory becoming unused. I have no idea if simply using "committed" pages would make the resident again, so I'd like to know what you think about this idea.

Also in a way I've been barking up the wrong tree, I should be asking the people in charge of the standard to open their minds to the possibility that maybe not everything can grow forever, that shrinking memories at least externally is something that could/should be possible and that the language of the standard should reflect that possibility.

@fitzgen
Copy link
Member

fitzgen commented Nov 7, 2024

I think adding something like a wasmtime::Memory::discard1 host API that is roughly equivalent to the proposed memory.discard instruction (i.e. semantically zeroes, always page aligned, always page multiple, uses the platform version of madvise(DONT_NEED) under the covers) is very doable. We will want to be careful with how we write the docs and all that so that its uses and trade offs are clear for users (including the implication of additional IPIs).

We wouldn't implement this for wasmtime::SharedMemory because of the atomicity issues on Windows.

And probably we would want it to fail for memories that use a custom page size.

Would this address your needs @Photosounder?

(Aside: this is a nice example of how being curious and open can lead to new ideas, potentially better than any initial proposal, that no one was originally considering. Thanks everyone for contributing to this thread!)

Footnotes

  1. Or maybe call it wasmtime::Memory::zero_and_decommit or something. Feel free to bikeshed...

@Photosounder
Copy link
Author

Photosounder commented Nov 7, 2024

I think adding something like a wasmtime::Memory::discard1 host API that is roughly equivalent to the proposed memory.discard instruction (i.e. semantically zeroes, always page aligned, always page multiple, uses the platform version of madvise(DONT_NEED) under the covers) is very doable. We will want to be careful with how we write the docs and all that so that its uses and trade offs are clear for users (including the implication of additional IPIs).

We wouldn't implement this for wasmtime::SharedMemory because of the atomicity issues on Windows.

And probably we would want it to fail for memories that use a custom page size.

Would this address your needs @Photosounder?

Yes this sounds like my latest suggestion, so this would suit me quite well in the sense that at least this would prevent large unused memories from being a practical problem. This just made me read about memory.discard again and I just realised that it isn't about shrinking the memory at all, which means there's no proposal that involves the possibility of actually shrinking memories. Now that I think of it I find it strange that a module could have an instruction like memory.discard to make the host OS do something like this, if I'm not mistaken it would make it quite unique in what it touches compared to other operands.

As for the name, if it follows the same principle as memory.discard it makes sense to call it the same, but "decommit" is more descriptive. I'd leave "zero_and_" out because it's perhaps more descriptive than it truly needs to be about how it works (I guess it's better to call a function after the general idea of its function rather than strictly what it does, and after all how it does things could change, but the idea behind it doesn't).

@fitzgen fitzgen changed the title Shrinking memory externally with wasmtime_memory_shrink() Add a wasmtime::Memory::discard function Nov 7, 2024
@fitzgen
Copy link
Member

fitzgen commented Nov 7, 2024

I've updated the issue title to reflect the new intentions here.

@Photosounder are you interested in trying your hand at implementing this feature?

@Photosounder
Copy link
Author

I've updated the issue title to reflect the new intentions here.

@Photosounder are you interested in trying your hand at implementing this feature?

Thanks, but I don't know Rust at all and like I said the topic of dealing with OS pages is new to me so it's best left to someone competent 😉

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