Skip to content

Commit

Permalink
Move dtor information from ResourceAny into a Store (#8061)
Browse files Browse the repository at this point in the history
* Move dtor information from `ResourceAny` into a `Store`

This commit moves the internals of the `ResourceAny` type related to
destructors into the auxiliary data within a `Store`. This avoids the
need to carry around pointers with `ResourceAny` 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 an `InstancePre` and
type information. This was required originally to get
`ResourceAny::resource_drop` working to drop the host resource. In
retrospect I'm not sure if this was the best goal to achieve because
`Resource<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` and `Resource<T>` pretty
close to "just an index" which is easier to reason about when working
with resources than carrying additional pointers.

* Remove now-unneeded ResourceImportIndex machinery
  • Loading branch information
alexcrichton authored Mar 8, 2024
1 parent 556fe42 commit dd3f8d8
Show file tree
Hide file tree
Showing 7 changed files with 138 additions and 249 deletions.
20 changes: 16 additions & 4 deletions crates/wasmtime/src/runtime/component/func/options.rs
Original file line number Diff line number Diff line change
Expand Up @@ -338,8 +338,14 @@ impl<'a, T> LowerContext<'a, T> {
///
/// Note that this is a special case for `Resource<T>`. Most of the time a
/// host value shouldn't be lowered with a lowering context.
pub fn host_resource_lower_own(&mut self, rep: u32) -> Result<HostResourceIndex> {
self.resource_tables().host_resource_lower_own(rep)
pub fn host_resource_lower_own(
&mut self,
rep: u32,
dtor: Option<NonNull<VMFuncRef>>,
flags: Option<InstanceFlags>,
) -> Result<HostResourceIndex> {
self.resource_tables()
.host_resource_lower_own(rep, dtor, flags)
}

/// Returns the underlying resource type for the `ty` table specified.
Expand Down Expand Up @@ -491,8 +497,14 @@ impl<'a> LiftContext<'a> {

/// Lowers a resource into the host-owned table, returning the index it was
/// inserted at.
pub fn host_resource_lower_own(&mut self, rep: u32) -> Result<HostResourceIndex> {
self.resource_tables().host_resource_lower_own(rep)
pub fn host_resource_lower_own(
&mut self,
rep: u32,
dtor: Option<NonNull<VMFuncRef>>,
flags: Option<InstanceFlags>,
) -> Result<HostResourceIndex> {
self.resource_tables()
.host_resource_lower_own(rep, dtor, flags)
}

/// Lowers a resource into the host-owned table, returning the index it was
Expand Down
20 changes: 1 addition & 19 deletions crates/wasmtime/src/runtime/component/instance.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
use crate::component::func::HostFunc;
use crate::component::matching::InstanceType;
use crate::component::{
Component, ComponentNamedList, Func, Lift, Lower, ResourceImportIndex, ResourceType, TypedFunc,
};
use crate::component::{Component, ComponentNamedList, Func, Lift, Lower, ResourceType, TypedFunc};
use crate::instance::OwnedImports;
use crate::linker::DefinitionType;
use crate::store::{StoreOpaque, Stored};
Expand Down Expand Up @@ -525,7 +523,6 @@ impl<'a> Instantiator<'a> {
pub struct InstancePre<T> {
component: Component,
imports: Arc<PrimaryMap<RuntimeImportIndex, RuntimeImport>>,
resource_imports: Arc<PrimaryMap<ResourceImportIndex, Option<RuntimeImportIndex>>>,
_marker: marker::PhantomData<fn() -> T>,
}

Expand All @@ -535,7 +532,6 @@ impl<T> Clone for InstancePre<T> {
Self {
component: self.component.clone(),
imports: self.imports.clone(),
resource_imports: self.resource_imports.clone(),
_marker: self._marker,
}
}
Expand All @@ -551,28 +547,14 @@ impl<T> InstancePre<T> {
pub(crate) unsafe fn new_unchecked(
component: Component,
imports: PrimaryMap<RuntimeImportIndex, RuntimeImport>,
resource_imports: PrimaryMap<ResourceImportIndex, Option<RuntimeImportIndex>>,
) -> InstancePre<T> {
InstancePre {
component,
imports: Arc::new(imports),
resource_imports: Arc::new(resource_imports),
_marker: marker::PhantomData,
}
}

pub(crate) fn resource_import_index(
&self,
path: ResourceImportIndex,
) -> Option<RuntimeImportIndex> {
*self.resource_imports.get(path)?
}

pub(crate) fn resource_import(&self, path: ResourceImportIndex) -> Option<&RuntimeImport> {
let idx = self.resource_import_index(path)?;
self.imports.get(idx)
}

/// Returns the underlying component that will be instantiated.
pub fn component(&self) -> &Component {
&self.component
Expand Down
80 changes: 11 additions & 69 deletions crates/wasmtime/src/runtime/component/linker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ use std::ops::Deref;
use std::pin::Pin;
use std::sync::Arc;
use wasmtime_environ::component::TypeDef;
use wasmtime_environ::{EntityRef, PrimaryMap};
use wasmtime_environ::PrimaryMap;

/// A type used to instantiate [`Component`]s.
///
Expand Down Expand Up @@ -64,7 +64,6 @@ pub struct Linker<T> {
strings: Strings,
map: NameMap,
path: Vec<usize>,
resource_imports: usize,
allow_shadowing: bool,
_marker: marker::PhantomData<fn() -> T>,
}
Expand All @@ -76,7 +75,6 @@ impl<T> Clone for Linker<T> {
strings: self.strings.clone(),
map: self.map.clone(),
path: self.path.clone(),
resource_imports: self.resource_imports.clone(),
allow_shadowing: self.allow_shadowing,
_marker: self._marker,
}
Expand All @@ -100,50 +98,10 @@ pub struct LinkerInstance<'a, T> {
path_len: usize,
strings: &'a mut Strings,
map: &'a mut NameMap,
resource_imports: &'a mut usize,
allow_shadowing: bool,
_marker: marker::PhantomData<fn() -> T>,
}

/// Index correlating a resource definition to the import path.
/// This is assigned by [`Linker::resource`] and may be used to associate it to
/// [`RuntimeImportIndex`](wasmtime_environ::component::RuntimeImportIndex)
/// at a later stage
///
/// [`Linker::resource`]: crate::component::LinkerInstance::resource
#[derive(Clone, Copy, Debug, Eq, Hash, PartialEq)]
pub struct ResourceImportIndex(usize);

impl EntityRef for ResourceImportIndex {
fn new(idx: usize) -> Self {
Self(idx)
}

fn index(self) -> usize {
self.0
}
}

impl Deref for ResourceImportIndex {
type Target = usize;

fn deref(&self) -> &Self::Target {
&self.0
}
}

impl From<usize> for ResourceImportIndex {
fn from(idx: usize) -> Self {
Self(idx)
}
}

impl From<ResourceImportIndex> for usize {
fn from(idx: ResourceImportIndex) -> Self {
idx.0
}
}

#[derive(Clone, Default)]
pub(crate) struct NameMap {
/// A map of interned strings to the name that they define.
Expand Down Expand Up @@ -184,11 +142,7 @@ pub(crate) enum Definition {
Instance(NameMap),
Func(Arc<HostFunc>),
Module(Module),
Resource(
ResourceImportIndex,
ResourceType,
Arc<crate::func::HostFunc>,
),
Resource(ResourceType, Arc<crate::func::HostFunc>),
}

impl<T> Linker<T> {
Expand All @@ -201,7 +155,6 @@ impl<T> Linker<T> {
map: NameMap::default(),
allow_shadowing: false,
path: Vec::new(),
resource_imports: 0,
_marker: marker::PhantomData,
}
}
Expand Down Expand Up @@ -229,7 +182,6 @@ impl<T> Linker<T> {
path_len: 0,
strings: &mut self.strings,
map: &mut self.map,
resource_imports: &mut self.resource_imports,
allow_shadowing: self.allow_shadowing,
_marker: self._marker,
}
Expand Down Expand Up @@ -304,7 +256,6 @@ impl<T> Linker<T> {
// component-compile-time.
let env_component = component.env_component();
let mut imports = PrimaryMap::with_capacity(env_component.imports.len());
let mut resource_imports = PrimaryMap::from(vec![None; self.resource_imports]);
for (idx, (import, names)) in env_component.imports.iter() {
let (root, _) = &env_component.import_types[*import];

Expand All @@ -321,14 +272,11 @@ impl<T> Linker<T> {
let import = match cur {
Definition::Module(m) => RuntimeImport::Module(m.clone()),
Definition::Func(f) => RuntimeImport::Func(f.clone()),
Definition::Resource(res_idx, t, dtor) => {
resource_imports[*res_idx] = Some(idx);
RuntimeImport::Resource {
ty: t.clone(),
_dtor: dtor.clone(),
dtor_funcref: component.resource_drop_func_ref(dtor),
}
}
Definition::Resource(t, dtor) => RuntimeImport::Resource {
ty: t.clone(),
_dtor: dtor.clone(),
dtor_funcref: component.resource_drop_func_ref(dtor),
},

// This is guaranteed by the compilation process that "leaf"
// runtime imports are never instances.
Expand All @@ -337,7 +285,7 @@ impl<T> Linker<T> {
let i = imports.push(import);
assert_eq!(i, idx);
}
Ok(unsafe { InstancePre::new_unchecked(component.clone(), imports, resource_imports) })
Ok(unsafe { InstancePre::new_unchecked(component.clone(), imports) })
}

/// Instantiates the [`Component`] provided into the `store` specified.
Expand Down Expand Up @@ -402,7 +350,6 @@ impl<T> LinkerInstance<'_, T> {
path_len: self.path_len,
strings: self.strings,
map: self.map,
resource_imports: self.resource_imports,
allow_shadowing: self.allow_shadowing,
_marker: self._marker,
}
Expand Down Expand Up @@ -581,18 +528,13 @@ impl<T> LinkerInstance<'_, T> {
name: &str,
ty: ResourceType,
dtor: impl Fn(StoreContextMut<'_, T>, u32) -> Result<()> + Send + Sync + 'static,
) -> Result<ResourceImportIndex> {
) -> Result<()> {
let dtor = Arc::new(crate::func::HostFunc::wrap(
&self.engine,
move |mut cx: crate::Caller<'_, T>, param: u32| dtor(cx.as_context_mut(), param),
));
let idx = ResourceImportIndex::new(*self.resource_imports);
*self.resource_imports = self
.resource_imports
.checked_add(1)
.context("resource import count would overflow")?;
self.insert(name, Definition::Resource(idx, ty, dtor))?;
Ok(idx)
self.insert(name, Definition::Resource(ty, dtor))?;
Ok(())
}

/// Defines a nested instance within this instance.
Expand Down
2 changes: 1 addition & 1 deletion crates/wasmtime/src/runtime/component/matching.rs
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ impl TypeChecker<'_> {
TypeDef::Resource(i) => {
let i = self.types[i].ty;
let actual = match actual {
Some(Definition::Resource(_idx, actual, _dtor)) => actual,
Some(Definition::Resource(actual, _dtor)) => actual,

// If a resource is imported yet nothing was supplied then
// that's only successful if the resource has itself
Expand Down
2 changes: 1 addition & 1 deletion crates/wasmtime/src/runtime/component/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ pub use self::func::{
ComponentNamedList, ComponentType, Func, Lift, Lower, TypedFunc, WasmList, WasmStr,
};
pub use self::instance::{ExportInstance, Exports, Instance, InstancePre};
pub use self::linker::{Linker, LinkerInstance, ResourceImportIndex};
pub use self::linker::{Linker, LinkerInstance};
pub use self::resource_table::{ResourceTable, ResourceTableError};
pub use self::resources::{Resource, ResourceAny};
pub use self::types::{ResourceType, Type};
Expand Down
Loading

0 comments on commit dd3f8d8

Please sign in to comment.