alexcrichton opened PR #8061 from alexcrichton:resource-just-an-index
to bytecodealliance:main
:
This commit moves the internals of the
ResourceAny
type related to destructors into the auxiliary data within aStore
. This avoids the need to carry around pointers withResourceAny
and additionally makes it easier to work with.As part of this commit this also updates the behavior of
ResourceAny::try_from_resource
to no longer need anInstancePre
and type information. This was required originally to getResourceAny::resource_drop
working to drop the host resource. In retrospect I'm not sure if this was the best goal to achieve becauseResource<T>
already has no destructor support and one of the more common use cases for the conversion is to simply turn around and give it back to a component where a component already has enough destructor information.In the end this should make both
ResourceAny
andResource<T>
pretty close to "just an index" which is easier to reason about when working with resources than carrying additional pointers.<!--
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 pchickey for a review on PR #8061.
alexcrichton requested wasmtime-core-reviewers for a review on PR #8061.
alexcrichton commented on PR #8061:
cc @rvolosatovs as this is touching on the work of https://github.com/bytecodealliance/wasmtime/pull/7688
github-actions[bot] commented on PR #8061:
Subscribe to Label Action
cc @peterhuene
<details>
This issue or pull request has been labeled: "wasmtime:api"Thus the following users have been cc'd because of the following labels:
- peterhuene: wasmtime:api
To subscribe or unsubscribe from this label, edit the <code>.github/subscribe-to-label.json</code> configuration file.
Learn more.
</details>
pchickey submitted PR review.
rvolosatovs submitted PR review:
Looks great, thanks.
ShouldResourceIndex
be removed altogether and not be returned anymore byLinkerInstance::resource
?It appears that this PR closes https://github.com/bytecodealliance/wasmtime/issues/7714
alexcrichton updated PR #8061.
alexcrichton edited PR #8061:
This commit moves the internals of the
ResourceAny
type related to destructors into the auxiliary data within aStore
. This avoids the need to carry around pointers withResourceAny
and additionally makes it easier to work with.As part of this commit this also updates the behavior of
ResourceAny::try_from_resource
to no longer need anInstancePre
and type information. This was required originally to getResourceAny::resource_drop
working to drop the host resource. In retrospect I'm not sure if this was the best goal to achieve becauseResource<T>
already has no destructor support and one of the more common use cases for the conversion is to simply turn around and give it back to a component where a component already has enough destructor information.In the end this should make both
ResourceAny
andResource<T>
pretty close to "just an index" which is easier to reason about when working with resources than carrying additional pointers.Closes #https://github.com/bytecodealliance/wasmtime/issues/7714
<!--
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 edited PR #8061:
This commit moves the internals of the
ResourceAny
type related to destructors into the auxiliary data within aStore
. This avoids the need to carry around pointers withResourceAny
and additionally makes it easier to work with.As part of this commit this also updates the behavior of
ResourceAny::try_from_resource
to no longer need anInstancePre
and type information. This was required originally to getResourceAny::resource_drop
working to drop the host resource. In retrospect I'm not sure if this was the best goal to achieve becauseResource<T>
already has no destructor support and one of the more common use cases for the conversion is to simply turn around and give it back to a component where a component already has enough destructor information.In the end this should make both
ResourceAny
andResource<T>
pretty close to "just an index" which is easier to reason about when working with resources than carrying additional pointers.Closes https://github.com/bytecodealliance/wasmtime/issues/7714
<!--
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 commented on PR #8061:
Good points! And sorry again for not realizing this is probably the best way to go sooner, I realize this is a bit of a runaround with the API :(
alexcrichton has enabled auto merge for PR #8061.
alexcrichton merged PR #8061.
Last updated: Dec 23 2024 at 13:07 UTC