Remove the wasmtime_environ::TablePlan type#9530
Merged
cfallin merged 2 commits intobytecodealliance:mainfrom Oct 31, 2024
Merged
Remove the wasmtime_environ::TablePlan type#9530cfallin merged 2 commits intobytecodealliance:mainfrom
wasmtime_environ::TablePlan type#9530cfallin merged 2 commits intobytecodealliance:mainfrom
Conversation
In my quest to simplify memory configuration and how things work internally in Wasmtime one thing I've identified to accomplish is the removal of the `TablePlan` and `MemoryPlan` types. These introduce an abstraction layer between table/memory implementations and `Tunables` and personally I find it simpler to directly reference `Tunables` and use that instead. The goal of this commit is to plumb `Tunables` closer to where it's directly read by removing the "indirection" through the `*Plan` types. The `TablePlan` and `MemoryPlan` types are pervasively used throughout Wasmtime so instead of having one large commit delete everything this is instead a piecemeal approach to incrementally get towards the goal of removal. Here just `TablePlan` is removed and `Tunables` is plumbed in a few more places. I plan to also in the future remove `TableStyle` and `MemoryStyle` in favor of directly reading `Tunables` but that's left for a future commit. For now `TableStyle` persists and its usage is a bit odd in isolation but I plan to follow this up with the removal of `TableStyle`.
alexcrichton
added a commit
to alexcrichton/wasmtime
that referenced
this pull request
Oct 31, 2024
This is a follow-up to bytecodealliance#9530 to remove the `wasmtime_environ::TableStyle` type. This type was added quite a long time ago for future-proofing other styles of tables but at this point it's simpler to remove the type as adding another style of table hasn't been on the "table" (heh) for quite some time. This intends to further the goal of bytecodealliance#9530 of plumbing `Tunables` closer to where it's ready to avoid extra layers of abstraction between the configuration options and what is processing them.
alexcrichton
added a commit
to alexcrichton/wasmtime
that referenced
this pull request
Oct 31, 2024
This is the equivalent of bytecodealliance#9530 for memories. The goal of this commit is to eventually remove the abstraction layer of `MemoryPlan` and `MemoryStyle` in favor of directly reading the configuration of `Tunables`. The prediction is that it will be simpler to work directly with configured values instead of a layer of abstraction between the configuration and the runtime which needs to be evolved independently to capture how to interpret the configuration. Like with bytecodealliance#9530 my plan is to eventually remove the `MemoryStyle` type itself, but that'll be a larger change, so it's deferred to a future PR.
github-merge-queue bot
pushed a commit
that referenced
this pull request
Oct 31, 2024
This is a follow-up to #9530 to remove the `wasmtime_environ::TableStyle` type. This type was added quite a long time ago for future-proofing other styles of tables but at this point it's simpler to remove the type as adding another style of table hasn't been on the "table" (heh) for quite some time. This intends to further the goal of #9530 of plumbing `Tunables` closer to where it's ready to avoid extra layers of abstraction between the configuration options and what is processing them.
alexcrichton
added a commit
to alexcrichton/wasmtime
that referenced
this pull request
Oct 31, 2024
This is a follow-up to bytecodealliance#9530 to remove the `wasmtime_environ::TableStyle` type. This type was added quite a long time ago for future-proofing other styles of tables but at this point it's simpler to remove the type as adding another style of table hasn't been on the "table" (heh) for quite some time. This intends to further the goal of bytecodealliance#9530 of plumbing `Tunables` closer to where it's ready to avoid extra layers of abstraction between the configuration options and what is processing them.
alexcrichton
added a commit
to alexcrichton/wasmtime
that referenced
this pull request
Oct 31, 2024
This is the equivalent of bytecodealliance#9530 for memories. The goal of this commit is to eventually remove the abstraction layer of `MemoryPlan` and `MemoryStyle` in favor of directly reading the configuration of `Tunables`. The prediction is that it will be simpler to work directly with configured values instead of a layer of abstraction between the configuration and the runtime which needs to be evolved independently to capture how to interpret the configuration. Like with bytecodealliance#9530 my plan is to eventually remove the `MemoryStyle` type itself, but that'll be a larger change, so it's deferred to a future PR.
github-merge-queue bot
pushed a commit
that referenced
this pull request
Oct 31, 2024
* Remove the `wasmtime_environ::MemoryPlan` type This is the equivalent of #9530 for memories. The goal of this commit is to eventually remove the abstraction layer of `MemoryPlan` and `MemoryStyle` in favor of directly reading the configuration of `Tunables`. The prediction is that it will be simpler to work directly with configured values instead of a layer of abstraction between the configuration and the runtime which needs to be evolved independently to capture how to interpret the configuration. Like with #9530 my plan is to eventually remove the `MemoryStyle` type itself, but that'll be a larger change, so it's deferred to a future PR. * Fix shared memory disabled build
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
In my quest to simplify memory configuration and how things work internally in Wasmtime one thing I've identified to accomplish is the removal of the
TablePlanandMemoryPlantypes. These introduce an abstraction layer between table/memory implementations andTunablesand personally I find it simpler to directly referenceTunablesand use that instead. The goal of this commit is to plumbTunablescloser to where it's directly read by removing the "indirection" through the*Plantypes.The
TablePlanandMemoryPlantypes are pervasively used throughout Wasmtime so instead of having one large commit delete everything this is instead a piecemeal approach to incrementally get towards the goal of removal. Here justTablePlanis removed andTunablesis plumbed in a few more places. I plan to also in the future removeTableStyleandMemoryStylein favor of directly readingTunablesbut that's left for a future commit. For nowTableStylepersists and its usage is a bit odd in isolation but I plan to follow this up with the removal ofTableStyle.