alexcrichton opened PR #3291 from changeup-opaque
to main
:
This commit is an overdue refactoring and renaming of some internals of
theStore
type in Wasmtime. The actual implementation ofStore<T>
has evolved from the original implementation to the point where some of
the aspects of how things are structured no longer makes sense. There's
also always been a lot of unnecessary gymnastics when trying to get
access to various store pieces depending on where you are inwasmtime
.This refactoring aims to simplify all this and make the internals much
easier to read/write. The following changes were made:
The
StoreOpaque<'_>
type is deleted, along with theopaque()
method.The
StoreInnermost
type was renamed toStoreOpaque
.
StoreOpaque<'_>
is dead. Long liveStoreOpaque
. This renaming
and a few small tweaks means that this type now suffices for all
consumers.The
AsContextMut
andAsContext
traits are now implemented for
StoreInner<T>
.These changes, while subtly small, help clean up a lot of the internals
ofwasmtime
. There's a lot less verbose&mut store.as_context_mut().opaque()
now. Additionally many methods can
simply start withlet store = store.as_context_mut().0;
and use things
internally. One of the nicer aspects of using references directly is
that the compiler automatically reborrows references as necessary
meaning there's lots of less manual reborrowing.The main motivation for this change was actually somewhat roundabout
where I found that whenStoreOpaque<'_>
was being captured in closures
and iterators it's 3 pointers wide which is a lot of data to move
around. Now things capture over&mut StoreOpaque
which is just one
nice and small pointer to move around. In any case though I've long
wanted to revisit the design of these internals to improve the
ergonomics. It's not expected that this change alone will really have
all that much impact on the performance ofwasmtime
.Finally a doc comment was added to
store.rs
to try to explain all the
Store
-related types since there are a nontrivial amount.<!--
Please ensure that the following steps are all taken care of before submitting
the PR.
[ ] This has been discussed in issue #..., or if not, please tell us why
here.[ ] A short description of what this does, why it is needed; if the
description becomes long, the matter should probably be discussed in an issue
first.[ ] This PR contains test cases, if meaningful.
- [ ] A reviewer from the core maintainer team has been assigned for this PR.
If you don't know who could review this, please indicate so. The list of
suggested reviewers on the right can help you.Please ensure all communication adheres to the code of conduct.
-->
alexcrichton updated PR #3291 from changeup-opaque
to main
.
alexcrichton requested pchickey for a review on PR #3291.
bjorn3 submitted PR review.
bjorn3 created PR review comment:
Can this be
pub(crate)
?
alexcrichton updated PR #3291 from changeup-opaque
to main
.
pchickey submitted PR review.
alexcrichton merged PR #3291.
alexcrichton submitted PR review.
alexcrichton created PR review comment:
Er forgot to answer this before I merged, but I don't think so. There's a lot of
pub
stuff already and I frequently get compiler errors about pub-crate things being in pub-interfaces, so I prefer to not deal with any of that.
Last updated: Dec 23 2024 at 12:05 UTC