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

Clear affine slots when dropping a Module #5321

Merged

Conversation

alexcrichton
Copy link
Member

This commit implements a resource usage optimization for Wasmtime with the pooling instance allocator by ensuring that when a Module is dropped its backing virtual memory mappings are all removed. Currently when a Module is dropped it releases a strong reference to its internal memory image but the memory image may stick around in individual pooling instance allocator slots. When using the Random allocation strategy, for example, this means that the memory images could stick around for a long time.

While not a pressing issue this has resource usage implications for Wasmtime. Namely removing a Module does not guarantee the memfd, if in use for a memory image, is closed and deallocated within the kernel. Unfortunately simply closing the memfd is not sufficient as well as the mappings into the address space additionally all need to be removed for the kernel to release the resources for the memfd. This means that to release all kernel-level resources for a Module all slots which have the memory image mapped in must have the slot reset.

This problem isn't particularly present when using the NextAvailable allocation strategy since the number of lingering memfds is proportional to the maximum concurrent size of wasm instances. With the Random and ReuseAffinity strategies, however, it's much more prominent because the number of lingering memfds can reach the total number of slots available. This can appear as a leak of kernel-level memory which can cause other system instability.

To fix this issue this commit adds necessary instrumentation to Drop for Module to purge all references to the module in the pooling instance allocator. All index allocation strategies now maintain affinity tracking to ensure that regardless of the strategy in use a module that is dropped will remove all its memory mappings. A new allocation method was added to the index allocator for allocating an index without setting affinity and only allocating affine slots. This is used to iterate over all the affine slots without holding the global index lock for an unnecessarily long time while mappings are removed.

This commit implements a resource usage optimization for Wasmtime with
the pooling instance allocator by ensuring that when a `Module` is
dropped its backing virtual memory mappings are all removed. Currently
when a `Module` is dropped it releases a strong reference to its
internal memory image but the memory image may stick around in
individual pooling instance allocator slots. When using the `Random`
allocation strategy, for example, this means that the memory images
could stick around for a long time.

While not a pressing issue this has resource usage implications for
Wasmtime. Namely removing a `Module` does not guarantee the memfd, if in
use for a memory image, is closed and deallocated within the kernel.
Unfortunately simply closing the memfd is not sufficient as well as the
mappings into the address space additionally all need to be removed for
the kernel to release the resources for the memfd. This means that to
release all kernel-level resources for a `Module` all slots which have
the memory image mapped in must have the slot reset.

This problem isn't particularly present when using the `NextAvailable`
allocation strategy since the number of lingering memfds is proportional
to the maximum concurrent size of wasm instances. With the `Random` and
`ReuseAffinity` strategies, however, it's much more prominent because
the number of lingering memfds can reach the total number of slots
available. This can appear as a leak of kernel-level memory which can
cause other system instability.

To fix this issue this commit adds necessary instrumentation to `Drop
for Module` to purge all references to the module in the pooling
instance allocator. All index allocation strategies now maintain
affinity tracking to ensure that regardless of the strategy in use a
module that is dropped will remove all its memory mappings. A new
allocation method was added to the index allocator for allocating an
index without setting affinity and only allocating affine slots. This is
used to iterate over all the affine slots without holding the global
index lock for an unnecessarily long time while mappings are removed.
@github-actions github-actions bot added the wasmtime:api Related to the API of the `wasmtime` crate itself label Nov 23, 2022
@github-actions
Copy link

Subscribe to Label Action

cc @peterhuene

This issue or pull request has been labeled: "wasmtime:api"

Thus the following users have been cc'd because of the following labels:

  • peterhuene: wasmtime:api

To subscribe or unsubscribe from this label, edit the .github/subscribe-to-label.json configuration file.

Learn more.

Copy link
Member

@cfallin cfallin left a comment

Choose a reason for hiding this comment

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

This looks correct to me -- the unification of the different strategies' state storage is nice too. I like that the "pick and clear the next slot with affinity to this module" operation is O(1) due to the freelist.

I have a few comments below to improve clarity but nothing critical!

@alexcrichton
Copy link
Member Author

Thanks for the review! I also fixed an issue where alloc_affine_and_clear_affinity wasn't working for anything other than ReuseAffinity which was also one of the motivations for the refactoring here.

@cfallin
Copy link
Member

cfallin commented Nov 24, 2022

Thanks for the review! I also fixed an issue where alloc_affine_and_clear_affinity wasn't working for anything other than ReuseAffinity which was also one of the motivations for the refactoring here.

Ah, good catch; updated diff LGTM.

@alexcrichton alexcrichton merged commit 951bdcb into bytecodealliance:main Nov 28, 2022
@alexcrichton alexcrichton deleted the clear-affine-memories branch November 28, 2022 14:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
wasmtime:api Related to the API of the `wasmtime` crate itself
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants