abrown edited PR #7364:
This change stems from how slicing memory slots into MPK-protected regions limits the number of memories each store can access: e.g., with fifteen keys in use, a store only has access to a fifteenth of the available slots. If we simply multiple the number of memory slots needed to run the
*.wast
spec tests by fifteen, we run out of available memory. This limits the number of protection keys used to two, which still allows us to test the functionality without reserving too much memory.Also, if Wasmtime is ever embedded in an application that also uses memory protection keys, it could be useful to limit how many Wasmtime allocates and uses.. This change not only limits the number of protection keys used at runtime, but takes that
further to attempt to limit the initial number of keys allocated. The unfortunate side effect of using aOnceLock
is that themax
setting is only applicable on the first invocation, the one that sets theOnceLock
.
abrown requested alexcrichton for a review on PR #7364.
abrown submitted PR review.
abrown created PR review comment:
Not sure about this?
abrown submitted PR review.
abrown created PR review comment:
@alexcrichton, I'm not sure I like how the
max
parameter is really only a hint: it only applies the first time one callskeys
.
abrown submitted PR review.
abrown created PR review comment:
I guess I should just move this logic into
keys
if we end up using this approach.
alexcrichton submitted PR review:
Seems reasonable to me! I think this is fine to start out with at least and we can iterate on it over time. I do agree with what you mentioned though of pushing the handling of
max
directly into thekeys
function.
alexcrichton submitted PR review:
Seems reasonable to me! I think this is fine to start out with at least and we can iterate on it over time. I do agree with what you mentioned though of pushing the handling of
max
directly into thekeys
function.
alexcrichton created PR review comment:
This makes some sense to me in that you doubled the number of memories which would halve the limit of concurrent tests because right now each test creates its own engine.
abrown updated PR #7364.
abrown has marked PR #7364 as ready for review.
abrown requested wasmtime-core-reviewers for a review on PR #7364.
abrown has enabled auto merge for PR #7364.
abrown updated PR #7364.
abrown merged PR #7364.
Last updated: Dec 23 2024 at 12:05 UTC