alexcrichton opened PR #6693 from alexcrichton:retain-only-one-pointer
to bytecodealliance:main
:
This commit updates how instantiation arguments are rooted within a component instance. Previously a list of import arguments was "unzipped" into a list of modules and a list of functions. This is becoming a bit more cumbersome in an upcoming change for resources where a third style of import is being added, so instead of this unzipping operation this commit instead replaces the list of imports with an
Arc
. This way instantiation is actually a bit cheaper since only oneArc
needs to be retained instead of each individual argument imported into a module.Additionally this refactors the way that exported modules are handled by avoiding translating everything into one list and instead continuing to store modules in two separate lists: those in the component and those imported.
<!--
Please make sure you include the following information:
If this work has been discussed elsewhere, please include a link to that
conversation. If it was discussed in an issue, just mention "issue #...".Explain why this change is needed. If the details are in an issue already,
this can be brief.Our development process is documented in the Wasmtime book:
https://docs.wasmtime.dev/contributing-development-process.htmlPlease ensure all communication follows the code of conduct:
https://github.com/bytecodealliance/wasmtime/blob/main/CODE_OF_CONDUCT.md
-->
alexcrichton requested jameysharp for a review on PR #6693.
alexcrichton requested wasmtime-core-reviewers for a review on PR #6693.
jameysharp submitted PR review:
I think this makes sense! I'm not absolutely sure I understand the surrounding context but this does appear to do what you said it should.
jameysharp submitted PR review:
I think this makes sense! I'm not absolutely sure I understand the surrounding context but this does appear to do what you said it should.
jameysharp created PR review comment:
Minor typo:
/// Strong references are stored to these arguments since pointers are saved
jameysharp created PR review comment:
Maybe grab a little more of this comment to add to the documentation for the
imports
field above? Assuming this explanation is still accurate with regard to what's being retained and why, I don't see anything else describing those details.
alexcrichton updated PR #6693.
alexcrichton created PR review comment:
Good point yeah, in general the component model docs, even on the internals, are long due for an update. This is a pattern used commonly throughout the core wasm implementation as well though (which also isn't necessarily thoroughly documented).
One difficulty is that it's not easy to tie the data structure's types to "oh yes here's the point where the
unsafe
block is ok" since the unsafety is so pervasive. Or at least right now with our current organization I'm not aware of a "single spot" but rather the whole codebase assumes a store lives forever and everything needed by anything in a store must be rooted in the store somehow.
alexcrichton has enabled auto merge for PR #6693.
alexcrichton merged PR #6693.
Last updated: Dec 23 2024 at 12:05 UTC