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

Refactor the internals of vm::Memory to simplify the RuntimeLinearMemory trait #9577

Open
wants to merge 10 commits into
base: main
Choose a base branch
from

Conversation

alexcrichton
Copy link
Member

This is a series of commits which have the end-goal of removing the RuntimeLinearMemory trait and replacing it with wasmtime::LinearMemory. In doing this though I wanted to simplify the trait to move more responsibilities up a layer so it's clearer what the safety contracts are and what exactly is required on behalf of embedders. This moves many of the responsibilities to a new LocalMemory type rather than having most of the functionality implemented through trait methods. This provides, for example, a vector of having CoW support in Wasmtime without supporting it through LinearMemory (or not officially at least).

Each commit here is probably best viewed one-at-a-time since this adds up to a number of changes but I've tried to keep things separated and have tests passing for each individual commit.

Fixes #9568

Instead of storing just a trait object store instead an `enum` between
"local" memory and shared memory. This helps remove redundant trait
impls on shared memories, removes the need for `dyn Any`, and removes
the possibility of wrapping a shared memory as a shared memory. This is
additionally intended to make it a bit easier to see what's nested where
and reduce some layers of indirection.
This is intended to be a non-shared implementation of a linear memory
with various bits and pieces tracking state. The end goal is to simplify
the `RuntimeLinearMemory` trait to its bare bones necessary to
faithfully implement wasm linear memory.
Further simplify `RuntimeLinearMemory` by moving these responsibilities
up a layer into the `LocalMemory` structure.
No need to plumb it through all the traits any more. Now it's possible
to only have it handled in `LocalMemory` and other pieces don't have to
worry about it.
Further simplify custom traits and mmap/static memory by moving more
responsibility into `LocalMemory`.
@alexcrichton alexcrichton requested a review from a team as a code owner November 6, 2024 22:05
@alexcrichton alexcrichton requested review from fitzgen and removed request for a team November 6, 2024 22:05
@alexcrichton alexcrichton requested a review from a team as a code owner November 6, 2024 22:38
@github-actions github-actions bot added fuzzing Issues related to our fuzzing infrastructure wasmtime:api Related to the API of the `wasmtime` crate itself wasmtime:c-api Issues pertaining to the C API. labels Nov 7, 2024
Copy link

github-actions bot commented Nov 7, 2024

Subscribe to Label Action

cc @fitzgen

This issue or pull request has been labeled: "fuzzing", "wasmtime:api", "wasmtime:c-api"

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

  • fitzgen: fuzzing

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

Learn more.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fuzzing Issues related to our fuzzing infrastructure wasmtime:api Related to the API of the `wasmtime` crate itself wasmtime:c-api Issues pertaining to the C API.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Refactor the MemoryCreator trait to support immovable memories
1 participant