Stream: git-wasmtime

Topic: wasmtime / PR #3837 Remove the `ModuleLimits` pooling con...

Wasmtime GitHub notifications bot (Feb 22 2022 at 23:43):

alexcrichton opened PR #3837 from no-module-limits to main:

This commit is an attempt to improve the usability of the pooling
allocator by removing the need to configure a ModuleLimits structure.
Internally this structure has limits on all forms of wasm constructs but
this largely bottoms out in the size of an allocation for an instance in
the instance pooling allocator. Maintaining this list of limits can be
cumbersome as modules may get tweaked over time and there's otherwise no
real reason to limit the number of globals in a module since the main
goal is to limit the memory consumption of a VMContext which can be
done with a memory allocation limit rather than fine-tuned control over
each maximum and minimum.

The new approach taken in this commit is to remove ModuleLimits. Some
fields, such as tables, table_elements , memories, and
memory_pages are moved to InstanceLimits since they're still
enforced at runtime. A new field size is added to InstanceLimits
which indicates, in bytes, the maximum size of the VMContext
allocation. If the size of a VMContext for a module exceeds this value
then instantiation will fail.

This involved adding a few more checks to {Table, Memory}::new_static
to ensure that the minimum size is able to fit in the allocation, since
previously modules were validated at compile time of the module that
everything fit and that validation no longer happens (it happens at
runtime).

A consequence of this commit is that Wasmtime will have no built-in way
to reject modules at compile time if they'll fail to be instantiated
within a particular pooling allocator configuration. Instead a module
must attempt instantiation see if a failure happens.

<!--

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 (Feb 22 2022 at 23:43):

alexcrichton requested peterhuene for a review on PR #3837.

Wasmtime GitHub notifications bot (Feb 23 2022 at 15:15):

alexcrichton updated PR #3837 from no-module-limits to main.

Wasmtime GitHub notifications bot (Feb 23 2022 at 16:51):

alexcrichton updated PR #3837 from no-module-limits to main.

Wasmtime GitHub notifications bot (Feb 23 2022 at 21:09):

peterhuene submitted PR review.

Wasmtime GitHub notifications bot (Feb 23 2022 at 21:09):

peterhuene submitted PR review.

Wasmtime GitHub notifications bot (Feb 23 2022 at 21:09):

peterhuene created PR review comment:

        // If using the pooling allocator, update the instance limits too

Wasmtime GitHub notifications bot (Feb 23 2022 at 21:09):

peterhuene created PR review comment:

Another nit:

This error message is a little confusing to me as it reads like the requirement is in the module and not that the module's initial memory size exceeded the maximum configured for the pooling allocator.

Perhaps initial memory size of {} bytes exceeds the pooling allocator's configured maximum memory size of {} bytes or something along those lines?

Wasmtime GitHub notifications bot (Feb 23 2022 at 21:09):

peterhuene created PR review comment:

nit:

                 exceeds the configured maximum of {}",

Wasmtime GitHub notifications bot (Feb 23 2022 at 21:09):

peterhuene created PR review comment:

Re: same confusing phrasing (to me at least) as used for Memory::new_static.

Wasmtime GitHub notifications bot (Feb 23 2022 at 23:21):

alexcrichton updated PR #3837 from no-module-limits to main.

Wasmtime GitHub notifications bot (Feb 24 2022 at 19:09):

alexcrichton updated PR #3837 from no-module-limits to main.

Wasmtime GitHub notifications bot (Feb 24 2022 at 20:06):

alexcrichton updated PR #3837 from no-module-limits to main.

Wasmtime GitHub notifications bot (Feb 24 2022 at 20:40):

alexcrichton requested peterhuene for a review on PR #3837.

Wasmtime GitHub notifications bot (Feb 24 2022 at 21:29):

peterhuene submitted PR review.

Wasmtime GitHub notifications bot (Feb 25 2022 at 15:11):

alexcrichton merged PR #3837.

Last updated: Nov 22 2024 at 17:03 UTC