Stream: git-wasmtime

Topic: wasmtime / PR #5321 Clear affine slots when dropping a `M...

Wasmtime GitHub notifications bot (Nov 23 2022 at 20:57):

alexcrichton opened PR #5321 from clear-affine-memories to main:

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.

<!--

Please ensure that the following steps are all taken care of before submitting
the PR.

• [ ] This has been discussed in issue #..., or if not, please tell us why
  here.

• [ ] A short description of what this does, why it is needed; if the
  description becomes long, the matter should probably be discussed in an issue
  first.

• [ ] This PR contains test cases, if meaningful.

• [ ] A reviewer from the core maintainer team has been assigned for this PR.
  If you don't know who could review this, please indicate so. The list of
  suggested reviewers on the right can help you.

Please ensure all communication adheres to the code of conduct.
-->

Wasmtime GitHub notifications bot (Nov 23 2022 at 22:31):

cfallin submitted PR review.

Wasmtime GitHub notifications bot (Nov 23 2022 at 22:31):

cfallin submitted PR review.

Wasmtime GitHub notifications bot (Nov 23 2022 at 22:31):

cfallin created PR review comment:

Can we make this a little indicator enum, e.g. enum AllocMode { ForceAffineAndClear, AnySlot } or somesuch?

Wasmtime GitHub notifications bot (Nov 23 2022 at 22:31):

cfallin created PR review comment:

pre-existing but can we rename this to module_id for clarity in the below?

Wasmtime GitHub notifications bot (Nov 23 2022 at 22:31):

cfallin created PR review comment:

s/bout/about/ ?

Wasmtime GitHub notifications bot (Nov 23 2022 at 22:31):

cfallin created PR review comment:

something about these two lines together feels confusing to me (though they are correct): the NextAvailable case picks but does not remove the index we're going to allocate, while alloc_random sounds like it allocates (i.e. removes from freelist).

Perhaps can we (i) add a comment before these two that we're going to remove from the freelist below, and (ii) rename the alloc_* helpers to pick_*, and (iii) use a helper for the first case here too?

Wasmtime GitHub notifications bot (Nov 24 2022 at 00:02):

alexcrichton updated PR #5321 from clear-affine-memories to main.

Wasmtime GitHub notifications bot (Nov 28 2022 at 14:58):

alexcrichton merged PR #5321.

Last updated: Nov 22 2024 at 16:03 UTC