Skip to content

Commit ab6d971

Browse files
committed
refactor(linker): only track import index for resources
Signed-off-by: Roman Volosatovs <rvolosatovs@riseup.net>
1 parent dba9850 commit ab6d971

5 files changed

Lines changed: 95 additions & 95 deletions

File tree

crates/wasmtime/src/component/instance.rs

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
use crate::component::func::HostFunc;
22
use crate::component::matching::InstanceType;
33
use crate::component::{
4-
Component, ComponentNamedList, Func, Lift, Lower, PathIndex, ResourceType, TypedFunc,
4+
Component, ComponentNamedList, Func, Lift, Lower, ResourceImportIndex, ResourceType, TypedFunc,
55
};
66
use crate::instance::OwnedImports;
77
use crate::linker::DefinitionType;
@@ -522,7 +522,7 @@ impl<'a> Instantiator<'a> {
522522
pub struct InstancePre<T> {
523523
component: Component,
524524
imports: Arc<PrimaryMap<RuntimeImportIndex, RuntimeImport>>,
525-
paths: Arc<Vec<Option<RuntimeImportIndex>>>,
525+
resource_imports: Arc<Vec<Option<RuntimeImportIndex>>>,
526526
_marker: marker::PhantomData<fn() -> T>,
527527
}
528528

@@ -532,7 +532,7 @@ impl<T> Clone for InstancePre<T> {
532532
Self {
533533
component: self.component.clone(),
534534
imports: self.imports.clone(),
535-
paths: self.paths.clone(),
535+
resource_imports: self.resource_imports.clone(),
536536
_marker: self._marker,
537537
}
538538
}
@@ -548,22 +548,25 @@ impl<T> InstancePre<T> {
548548
pub(crate) unsafe fn new_unchecked(
549549
component: Component,
550550
imports: PrimaryMap<RuntimeImportIndex, RuntimeImport>,
551-
paths: Vec<Option<RuntimeImportIndex>>,
551+
resource_imports: Vec<Option<RuntimeImportIndex>>,
552552
) -> InstancePre<T> {
553553
InstancePre {
554554
component,
555555
imports: Arc::new(imports),
556-
paths: Arc::new(paths),
556+
resource_imports: Arc::new(resource_imports),
557557
_marker: marker::PhantomData,
558558
}
559559
}
560560

561-
pub(crate) fn runtime_import_index(&self, path: PathIndex) -> Option<RuntimeImportIndex> {
562-
*self.paths.get(*path)?
561+
pub(crate) fn resource_import_index(
562+
&self,
563+
path: ResourceImportIndex,
564+
) -> Option<RuntimeImportIndex> {
565+
*self.resource_imports.get(*path)?
563566
}
564567

565-
pub(crate) fn import_by_path(&self, path: PathIndex) -> Option<&RuntimeImport> {
566-
let idx = self.runtime_import_index(path)?;
568+
pub(crate) fn resource_import(&self, path: ResourceImportIndex) -> Option<&RuntimeImport> {
569+
let idx = self.resource_import_index(path)?;
567570
self.imports.get(idx)
568571
}
569572

crates/wasmtime/src/component/linker.rs

Lines changed: 66 additions & 72 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ pub struct Linker<T> {
2727
strings: Strings,
2828
map: NameMap,
2929
path: Vec<usize>,
30-
paths: Vec<Vec<usize>>, // all paths defined in this linker
30+
resource_imports: usize,
3131
allow_shadowing: bool,
3232
_marker: marker::PhantomData<fn() -> T>,
3333
}
@@ -39,7 +39,7 @@ impl<T> Clone for Linker<T> {
3939
strings: self.strings.clone(),
4040
map: self.map.clone(),
4141
path: self.path.clone(),
42-
paths: self.paths.clone(),
42+
resource_imports: self.resource_imports.clone(),
4343
allow_shadowing: self.allow_shadowing,
4444
_marker: self._marker,
4545
}
@@ -63,43 +63,35 @@ pub struct LinkerInstance<'a, T> {
6363
path_len: usize,
6464
strings: &'a mut Strings,
6565
map: &'a mut NameMap,
66-
paths: &'a mut Vec<Vec<usize>>,
66+
resource_imports: &'a mut usize,
6767
allow_shadowing: bool,
6868
_marker: marker::PhantomData<fn() -> T>,
6969
}
7070

71-
/// Index correlating a linker item definition to the import path
71+
/// Index correlating a resource definition to the import path
7272
#[derive(Clone, Copy, Debug, Eq, Hash, PartialEq)]
73-
pub struct PathIndex(usize);
73+
pub struct ResourceImportIndex(usize);
7474

75-
impl Deref for PathIndex {
75+
impl Deref for ResourceImportIndex {
7676
type Target = usize;
7777

7878
fn deref(&self) -> &Self::Target {
7979
&self.0
8080
}
8181
}
8282

83-
impl PathIndex {
84-
fn key(paths: &mut Vec<Vec<usize>>, path: &[usize], key: usize) -> Self {
85-
let idx = Self(paths.len());
86-
let len = path.len();
87-
let mut key_path = vec![key; len + 1];
88-
let (head, _) = key_path.split_at_mut(len);
89-
head.copy_from_slice(path);
90-
paths.push(key_path);
91-
idx
92-
}
93-
}
94-
95-
pub(crate) type NameMap = HashMap<usize, (PathIndex, Definition)>;
83+
pub(crate) type NameMap = HashMap<usize, Definition>;
9684

9785
#[derive(Clone)]
9886
pub(crate) enum Definition {
9987
Instance(NameMap),
10088
Func(Arc<HostFunc>),
10189
Module(Module),
102-
Resource(ResourceType, Arc<crate::func::HostFunc>),
90+
Resource(
91+
ResourceImportIndex,
92+
ResourceType,
93+
Arc<crate::func::HostFunc>,
94+
),
10395
}
10496

10597
impl<T> Linker<T> {
@@ -112,7 +104,7 @@ impl<T> Linker<T> {
112104
map: NameMap::default(),
113105
allow_shadowing: false,
114106
path: Vec::new(),
115-
paths: Vec::new(),
107+
resource_imports: 0,
116108
_marker: marker::PhantomData,
117109
}
118110
}
@@ -140,7 +132,7 @@ impl<T> Linker<T> {
140132
path_len: 0,
141133
strings: &mut self.strings,
142134
map: &mut self.map,
143-
paths: &mut self.paths,
135+
resource_imports: &mut self.resource_imports,
144136
allow_shadowing: self.allow_shadowing,
145137
_marker: self._marker,
146138
}
@@ -189,8 +181,7 @@ impl<T> Linker<T> {
189181
let import = self
190182
.strings
191183
.lookup(name)
192-
.and_then(|name| self.map.get(&name))
193-
.map(|(_, import)| import);
184+
.and_then(|name| self.map.get(&name));
194185
cx.definition(ty, import)
195186
.with_context(|| format!("import `{name}` has the wrong type"))?;
196187
}
@@ -200,44 +191,42 @@ impl<T> Linker<T> {
200191
// using the import map within the component created at
201192
// component-compile-time.
202193
let mut imports = PrimaryMap::with_capacity(env_component.imports.len());
203-
let mut paths = vec![None; self.paths.len()];
204-
for (idx, (import, path)) in env_component.imports.iter() {
194+
let mut resource_imports = vec![None; self.resource_imports];
195+
for (idx, (import, names)) in env_component.imports.iter() {
205196
let (root, _) = &env_component.import_types[*import];
206197
let root = self.strings.lookup(root).unwrap();
207198

208199
// This is the flattening process where we go from a definition
209200
// optionally through a list of exported names to get to the final
210201
// item.
211-
let (path_idx, def) = &self.map[&root];
212-
let mut path_idx = path_idx;
213-
let mut def = def;
214-
for name in path {
202+
let mut cur = &self.map[&root];
203+
for name in names {
215204
let name = self.strings.lookup(name).unwrap();
216-
let (cur_path_idx, cur_def) = match def {
205+
cur = match cur {
217206
Definition::Instance(map) => &map[&name],
218207
_ => unreachable!(),
219208
};
220-
path_idx = cur_path_idx;
221-
def = cur_def;
222209
}
223-
let import = match def {
210+
let import = match cur {
224211
Definition::Module(m) => RuntimeImport::Module(m.clone()),
225212
Definition::Func(f) => RuntimeImport::Func(f.clone()),
226-
Definition::Resource(t, dtor) => RuntimeImport::Resource {
227-
ty: t.clone(),
228-
_dtor: dtor.clone(),
229-
dtor_funcref: component.resource_drop_func_ref(dtor),
230-
},
213+
Definition::Resource(res_idx, t, dtor) => {
214+
resource_imports[*res_idx.deref()] = Some(idx);
215+
RuntimeImport::Resource {
216+
ty: t.clone(),
217+
_dtor: dtor.clone(),
218+
dtor_funcref: component.resource_drop_func_ref(dtor),
219+
}
220+
}
231221

232222
// This is guaranteed by the compilation process that "leaf"
233223
// runtime imports are never instances.
234-
Definition::Instance(..) => unreachable!(),
224+
Definition::Instance(_) => unreachable!(),
235225
};
236226
let i = imports.push(import);
237-
paths[*path_idx.deref()] = Some(i);
238227
assert_eq!(i, idx);
239228
}
240-
Ok(unsafe { InstancePre::new_unchecked(component.clone(), imports, paths) })
229+
Ok(unsafe { InstancePre::new_unchecked(component.clone(), imports, resource_imports) })
241230
}
242231

243232
/// Instantiates the [`Component`] provided into the `store` specified.
@@ -302,7 +291,7 @@ impl<T> LinkerInstance<'_, T> {
302291
path_len: self.path_len,
303292
strings: self.strings,
304293
map: self.map,
305-
paths: self.paths,
294+
resource_imports: self.resource_imports,
306295
allow_shadowing: self.allow_shadowing,
307296
_marker: self._marker,
308297
}
@@ -326,7 +315,7 @@ impl<T> LinkerInstance<'_, T> {
326315
/// argument which can be provided to the `func` given here.
327316
//
328317
// TODO: needs more words and examples
329-
pub fn func_wrap<F, Params, Return>(&mut self, name: &str, func: F) -> Result<PathIndex>
318+
pub fn func_wrap<F, Params, Return>(&mut self, name: &str, func: F) -> Result<()>
330319
where
331320
F: Fn(StoreContextMut<T>, Params) -> Result<Return> + Send + Sync + 'static,
332321
Params: ComponentNamedList + Lift + 'static,
@@ -342,7 +331,7 @@ impl<T> LinkerInstance<'_, T> {
342331
/// host function.
343332
#[cfg(feature = "async")]
344333
#[cfg_attr(nightlydoc, doc(cfg(feature = "async")))]
345-
pub fn func_wrap_async<Params, Return, F>(&mut self, name: &str, f: F) -> Result<PathIndex>
334+
pub fn func_wrap_async<Params, Return, F>(&mut self, name: &str, f: F) -> Result<()>
346335
where
347336
F: for<'a> Fn(
348337
StoreContextMut<'a, T>,
@@ -379,7 +368,7 @@ impl<T> LinkerInstance<'_, T> {
379368
component: &Component,
380369
name: &str,
381370
func: F,
382-
) -> Result<PathIndex> {
371+
) -> Result<()> {
383372
let mut map = &component
384373
.env_component()
385374
.import_types
@@ -421,12 +410,7 @@ impl<T> LinkerInstance<'_, T> {
421410
/// host function.
422411
#[cfg(feature = "async")]
423412
#[cfg_attr(nightlydoc, doc(cfg(feature = "async")))]
424-
pub fn func_new_async<F>(
425-
&mut self,
426-
component: &Component,
427-
name: &str,
428-
f: F,
429-
) -> Result<PathIndex>
413+
pub fn func_new_async<F>(&mut self, component: &Component, name: &str, f: F) -> Result<()>
430414
where
431415
F: for<'a> Fn(
432416
StoreContextMut<'a, T>,
@@ -454,7 +438,7 @@ impl<T> LinkerInstance<'_, T> {
454438
/// This can be used to provide a core wasm [`Module`] as an import to a
455439
/// component. The [`Module`] provided is saved within the linker for the
456440
/// specified `name` in this instance.
457-
pub fn module(&mut self, name: &str, module: &Module) -> Result<PathIndex> {
441+
pub fn module(&mut self, name: &str, module: &Module) -> Result<()> {
458442
let name = self.strings.intern(name);
459443
self.insert(name, Definition::Module(module.clone()))
460444
}
@@ -486,13 +470,31 @@ impl<T> LinkerInstance<'_, T> {
486470
name: &str,
487471
ty: ResourceType,
488472
dtor: impl Fn(StoreContextMut<'_, T>, u32) -> Result<()> + Send + Sync + 'static,
489-
) -> Result<PathIndex> {
473+
) -> Result<ResourceImportIndex> {
490474
let name = self.strings.intern(name);
491475
let dtor = Arc::new(crate::func::HostFunc::wrap(
492476
&self.engine,
493477
move |mut cx: crate::Caller<'_, T>, param: u32| dtor(cx.as_context_mut(), param),
494478
));
495-
self.insert(name, Definition::Resource(ty, dtor))
479+
let entry = self.map.entry(name);
480+
if !self.allow_shadowing && matches!(entry, Entry::Occupied(_)) {
481+
bail!("import of `{}` defined twice", self.strings.strings[name])
482+
}
483+
let idx = ResourceImportIndex(*self.resource_imports);
484+
*self.resource_imports = self
485+
.resource_imports
486+
.checked_add(1)
487+
.context("resource import count would overflow")?;
488+
let def = Definition::Resource(idx, ty, dtor);
489+
match entry {
490+
Entry::Occupied(mut o) => {
491+
o.insert(def);
492+
}
493+
Entry::Vacant(v) => {
494+
v.insert(def);
495+
}
496+
}
497+
Ok(idx)
496498
}
497499

498500
/// Defines a nested instance within this instance.
@@ -513,15 +515,11 @@ impl<T> LinkerInstance<'_, T> {
513515
bail!("import of `{}` defined twice", self.strings.strings[name])
514516
}
515517
Entry::Occupied(o) => {
516-
let (_, slot) = o.into_mut();
518+
let slot = o.into_mut();
517519
*slot = item;
518520
slot
519521
}
520-
Entry::Vacant(v) => {
521-
let idx = PathIndex::key(&mut self.paths, &self.path, name);
522-
let (_, slot) = v.insert((idx, item));
523-
slot
524-
}
522+
Entry::Vacant(v) => v.insert(item),
525523
};
526524
self.map = match slot {
527525
Definition::Instance(map) => map,
@@ -533,23 +531,19 @@ impl<T> LinkerInstance<'_, T> {
533531
Ok(self)
534532
}
535533

536-
fn insert(&mut self, key: usize, item: Definition) -> Result<PathIndex> {
537-
let idx = match self.map.entry(key) {
534+
fn insert(&mut self, key: usize, item: Definition) -> Result<()> {
535+
match self.map.entry(key) {
538536
Entry::Occupied(_) if !self.allow_shadowing => {
539537
bail!("import of `{}` defined twice", self.strings.strings[key])
540538
}
541-
Entry::Occupied(e) => {
542-
let (idx, slot) = e.into_mut();
543-
*slot = item;
544-
*idx
539+
Entry::Occupied(mut e) => {
540+
e.insert(item);
545541
}
546542
Entry::Vacant(v) => {
547-
let idx = PathIndex::key(&mut self.paths, &self.path, key);
548-
v.insert((idx, item));
549-
idx
543+
v.insert(item);
550544
}
551-
};
552-
Ok(idx)
545+
}
546+
Ok(())
553547
}
554548
}
555549

crates/wasmtime/src/component/matching.rs

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ impl TypeChecker<'_> {
5353
TypeDef::Resource(i) => {
5454
let i = self.types[i].ty;
5555
let actual = match actual {
56-
Some(Definition::Resource(actual, _dtor)) => actual,
56+
Some(Definition::Resource(_idx, actual, _dtor)) => actual,
5757

5858
// If a resource is imported yet nothing was supplied then
5959
// that's only successful if the resource has itself
@@ -152,8 +152,7 @@ impl TypeChecker<'_> {
152152
let actual = self
153153
.strings
154154
.lookup(name)
155-
.and_then(|name| actual?.get(&name))
156-
.map(|(_, actual)| actual);
155+
.and_then(|name| actual?.get(&name));
157156
self.definition(expected, actual)
158157
.with_context(|| format!("instance export `{name}` has the wrong type"))?;
159158
}

crates/wasmtime/src/component/mod.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ pub use self::func::{
2121
ComponentNamedList, ComponentType, Func, Lift, Lower, TypedFunc, WasmList, WasmStr,
2222
};
2323
pub use self::instance::{ExportInstance, Exports, Instance, InstancePre};
24-
pub use self::linker::{Linker, LinkerInstance, PathIndex};
24+
pub use self::linker::{Linker, LinkerInstance, ResourceImportIndex};
2525
pub use self::resource_table::{ResourceTable, ResourceTableError};
2626
pub use self::resources::{Resource, ResourceAny};
2727
pub use self::types::{ResourceType, Type};

0 commit comments

Comments
 (0)