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 aModuleLimits
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 aVMContext
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 astables
,table_elements
,memories
, and
memory_pages
are moved toInstanceLimits
since they're still
enforced at runtime. A new fieldsize
is added toInstanceLimits
which indicates, in bytes, the maximum size of theVMContext
allocation. If the size of aVMContext
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.
-->
alexcrichton requested peterhuene for a review on PR #3837.
alexcrichton updated PR #3837 from no-module-limits
to main
.
alexcrichton updated PR #3837 from no-module-limits
to main
.
peterhuene submitted PR review.
peterhuene submitted PR review.
peterhuene created PR review comment:
// If using the pooling allocator, update the instance limits too
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?
peterhuene created PR review comment:
nit:
exceeds the configured maximum of {}",
peterhuene created PR review comment:
Re: same confusing phrasing (to me at least) as used for
Memory::new_static
.
alexcrichton updated PR #3837 from no-module-limits
to main
.
alexcrichton updated PR #3837 from no-module-limits
to main
.
alexcrichton updated PR #3837 from no-module-limits
to main
.
alexcrichton requested peterhuene for a review on PR #3837.
peterhuene submitted PR review.
alexcrichton merged PR #3837.
Last updated: Dec 23 2024 at 12:05 UTC