diff --git a/crates/js-component-bindgen/src/intrinsics/mod.rs b/crates/js-component-bindgen/src/intrinsics/mod.rs index 813ea3db4..52433ec1e 100644 --- a/crates/js-component-bindgen/src/intrinsics/mod.rs +++ b/crates/js-component-bindgen/src/intrinsics/mod.rs @@ -92,7 +92,6 @@ pub enum Intrinsic { SymbolAsyncIterator, SymbolIterator, ScopeId, - DefinedResourceTables, HandleTables, /// Class that conforms to a `ReadableStreams`-like interface and is usable externally @@ -293,8 +292,6 @@ impl Intrinsic { ", ), - Intrinsic::DefinedResourceTables => {} - Intrinsic::FinalizationRegistryCreate => output.push_str( " function finalizationRegistryCreate (unregister) { @@ -358,11 +355,15 @@ impl Intrinsic { Intrinsic::WebIdl(w) => w.render(output), - Intrinsic::HandleTables => output.push_str( - " - const handleTables = []; - ", - ), + Intrinsic::HandleTables => { + let var_name = self.name(); + uwriteln!( + output, + r#" + const {var_name} = []; + "#, + ); + } Intrinsic::HasOwnProperty => output.push_str( " @@ -1636,11 +1637,10 @@ impl Intrinsic { "base64Compile", "clampGuest", "ComponentError", - "definedResourceTables", "fetchCompile", "finalizationRegistryCreate", "getErrorPayload", - "handleTables", + "HANDLE_TABLES", "hasOwnProperty", "imports", "instantiateCore", @@ -1702,12 +1702,11 @@ impl Intrinsic { Intrinsic::Base64Compile => "base64Compile", Intrinsic::ClampGuest => "clampGuest", Intrinsic::ComponentError => "ComponentError", - Intrinsic::DefinedResourceTables => "definedResourceTables", Intrinsic::FetchCompile => "fetchCompile", Intrinsic::FinalizationRegistryCreate => "finalizationRegistryCreate", Intrinsic::GetErrorPayload => "getErrorPayload", Intrinsic::GetErrorPayloadString => "getErrorPayloadString", - Intrinsic::HandleTables => "handleTables", + Intrinsic::HandleTables => "HANDLE_TABLES", Intrinsic::HasOwnProperty => "hasOwnProperty", Intrinsic::InstantiateCore => "instantiateCore", Intrinsic::IsLE => "isLE", diff --git a/crates/js-component-bindgen/src/intrinsics/resource.rs b/crates/js-component-bindgen/src/intrinsics/resource.rs index 6bc57f205..9c5254a60 100644 --- a/crates/js-component-bindgen/src/intrinsics/resource.rs +++ b/crates/js-component-bindgen/src/intrinsics/resource.rs @@ -1,7 +1,10 @@ //! Intrinsics that represent helpers that deal with Component Model resources +use std::fmt::Write; + use crate::intrinsics::{Intrinsic, RenderIntrinsicsArgs}; use crate::source::Source; +use crate::uwriteln; /// This enum contains intrinsics for supporting Component Model resources #[derive(Debug, Copy, Clone, Ord, PartialOrd, Eq, PartialEq)] @@ -118,28 +121,32 @@ impl ResourceIntrinsic { ", ), - Self::ResourceTableCreateBorrow => output.push_str( - r#" - function rscTableCreateBorrow(table, rep, scopeId) { - if (scopeId === undefined) {{ throw new Error("missing scopeId"); }} - const free = table[0] & ~T_FLAG; - if (free === 0) { - table.push(scopeId); - table.push(rep); - return (table.length >> 1) - 1; - } - table[0] = table[free]; - table[free << 1] = scopeId; - table[(free << 1) + 1] = rep; - return free; - } - "#, - ), + Self::ResourceTableCreateBorrow => { + uwriteln!( + output, + r#" + function rscTableCreateBorrow(table, rep, scopeId) {{ + if (scopeId === undefined) {{ throw new Error("missing scopeId"); }} + const free = table[0] & ~T_FLAG; + if (free === 0) {{ + table.push(scopeId); + table.push(rep); + return (table.length >> 1) - 1; + }} + table[0] = table[free << 1]; + table[free << 1] = scopeId; + table[(free << 1) + 1] = rep; + return free; + }} + "#, + ); + } Self::ResourceTableCreateOwn => output.push_str( " function rscTableCreateOwn(table, rep) { const free = table[0] & ~T_FLAG; + table._createdReps.add(rep); if (free === 0) { table.push(0); table.push(rep | T_FLAG); @@ -196,56 +203,89 @@ impl ResourceIntrinsic { ), Self::ResourceTransferBorrow => { + let resource_transfer_borrow_fn = self.name(); let handle_tables = Intrinsic::HandleTables.name(); let resource_borrows = Self::ResourceCallBorrows.name(); let rsc_table_remove = Self::ResourceTableRemove.name(); let rsc_table_create_borrow = Self::ResourceTableCreateBorrow.name(); - let defined_resource_tables = Intrinsic::DefinedResourceTables.name(); let scope_id = Intrinsic::ScopeId.name(); - output.push_str(&format!(" - function resourceTransferBorrow(handle, fromTid, toTid) {{ + + uwriteln!( + output, + r#" + function {resource_transfer_borrow_fn}(handle, fromTid, toTid) {{ const fromTable = {handle_tables}[fromTid]; const fromHandle = fromTable[(handle << 1) + 1]; const isOwn = (fromHandle & T_FLAG) !== 0; const rep = isOwn ? fromHandle & ~T_FLAG : {rsc_table_remove}(fromTable, fromHandle).rep; - if ({defined_resource_tables}[toTid]) return rep; - const toTable = {handle_tables}[toTid] || ({handle_tables}[toTid] = [T_FLAG, 0]); + + let toTable = {handle_tables}[toTid]; + if (!toTable) {{ + {handle_tables}[toTid] = [T_FLAG, 0]; + toTable = {handle_tables}[toTid]; + toTable._createdReps = new Set(); + }} + + if (toTable._createdReps.has(rep)) {{ + return rep; + }} + const newHandle = {rsc_table_create_borrow}(toTable, rep, {scope_id}); {resource_borrows}.push({{ rid: toTid, handle: newHandle }}); return newHandle; }} - ")); + "# + ); } Self::ResourceTransferBorrowValidLifting => { let handle_tables = Intrinsic::HandleTables.name(); let rsc_table_remove = Self::ResourceTableRemove.name(); let rsc_table_create_borrow = Self::ResourceTableCreateBorrow.name(); - let defined_resource_tables = Intrinsic::DefinedResourceTables.name(); let scope_id = Intrinsic::ScopeId.name(); - output.push_str(&format!(" + output.push_str(&format!(r#" function resourceTransferBorrowValidLifting(handle, fromTid, toTid) {{ const fromTable = {handle_tables}[fromTid]; const isOwn = (fromTable[(handle << 1) + 1] & T_FLAG) !== 0; const rep = isOwn ? fromTable[(handle << 1) + 1] & ~T_FLAG : {rsc_table_remove}(fromTable, handle).rep; - if ({defined_resource_tables}[toTid]) return rep; - const toTable = {handle_tables}[toTid] || ({handle_tables}[toTid] = [T_FLAG, 0]); + + let toTable = {handle_tables}[toTid]; + if (!toTable) {{ + {handle_tables}[toTid] = [T_FLAG, 0]; + toTable = {handle_tables}[toTid]; + toTable._createdReps = new Set(); + }} + + if (toTable._createdReps.has(rep)) {{ + return rep; + }} + return {rsc_table_create_borrow}(toTable, rep, {scope_id}); }} - ")); + "#)); } Self::ResourceTransferOwn => { let handle_tables = Intrinsic::HandleTables.name(); let rsc_table_remove = Self::ResourceTableRemove.name(); let rsc_table_create_own = Self::ResourceTableCreateOwn.name(); - output.push_str(&format!(" + output.push_str(&format!( + r#" function resourceTransferOwn(handle, fromTid, toTid) {{ const {{ rep }} = {rsc_table_remove}({handle_tables}[fromTid], handle); - const toTable = {handle_tables}[toTid] || ({handle_tables}[toTid] = [T_FLAG, 0]); - return {rsc_table_create_own}(toTable, rep); + + let toTable = {handle_tables}[toTid]; + if (!toTable) {{ + {handle_tables}[toTid] = [T_FLAG, 0]; + toTable = {handle_tables}[toTid]; + toTable._createdReps = new Set(); + }} + + const newHandle = {rsc_table_create_own}(toTable, rep); + return newHandle; }} - ")); + "# + )); } Self::ResourceCallBorrows => { diff --git a/crates/js-component-bindgen/src/transpile_bindgen.rs b/crates/js-component-bindgen/src/transpile_bindgen.rs index d00066cf8..4dd497ebd 100644 --- a/crates/js-component-bindgen/src/transpile_bindgen.rs +++ b/crates/js-component-bindgen/src/transpile_bindgen.rs @@ -323,9 +323,7 @@ pub fn transpile_bindgen( instantiator.initialize(); instantiator.instantiate(); - let mut intrinsic_definitions = source::Source::default(); - - instantiator.resource_definitions(&mut intrinsic_definitions); + instantiator.resource_definitions(); instantiator.instance_flags(); instantiator.bindgen.src.js(&instantiator.src.js); @@ -333,7 +331,7 @@ pub fn transpile_bindgen( instantiator .bindgen - .finish_component(name, files, &opts, intrinsic_definitions); + .finish_component(name, files, &opts, source::Source::default()); let exports = instantiator .bindgen @@ -956,7 +954,7 @@ impl<'a> Instantiator<'a, '_> { } } - fn resource_definitions(&mut self, definitions: &mut source::Source) { + fn resource_definitions(&mut self) { // It is theoretically possible for locally defined resources used in no functions // to still be exported for resource in 0..self.component.num_resources { @@ -970,30 +968,21 @@ impl<'a> Instantiator<'a, '_> { } } - // Write out the defined resource table indices for the runtime - if self.bindgen.all_intrinsics.contains(&Intrinsic::Resource( - ResourceIntrinsic::ResourceTransferBorrow, - )) || self.bindgen.all_intrinsics.contains(&Intrinsic::Resource( - ResourceIntrinsic::ResourceTransferBorrowValidLifting, - )) { - let defined_resource_tables = Intrinsic::DefinedResourceTables.name(); - uwrite!(definitions, "const {defined_resource_tables} = ["); - // Table per-resource - for tidx in 0..self.component.num_resources { - let tid = TypeResourceTableIndex::from_u32(tidx); - let resource_table_ty = &self.types[tid]; - let rid = resource_table_ty.unwrap_concrete_ty(); - if let Some(defined_index) = self.component.defined_resource_index(rid) { - let instance_idx = resource_table_ty.unwrap_concrete_instance(); - if instance_idx == self.component.defined_resource_instances[defined_index] { - uwrite!(definitions, "true,"); - } - } else { - uwrite!(definitions, ","); - }; - } - uwrite!(definitions, "];\n"); - } + // TODO(feat): In the past, we could eagerly build a mapping of resources defined to the tables they correspond to + // to make runtime checks of which table a resource must have been created into first (i.e. the component that implements + // creation of a given resource) a particular resource faster. + // + // This logic was based on component.num_resources and was broken for composed components -- + // wasmtime-environ reported a smaller number of resources, and the check was geared towards a *single* component, + // not tying a particular component to a particular table (it is possible for *sub components* to originate different + // resources). + // + // In theory, it should be posible to build this knowledge statically rather than at resource creation, + // so in the future we should attempt to rebulid that code, if possible. + // + // Note that at runtime wasmtime maintains this information and looks it up during a transfer operation, see: + // - https://github.com/bytecodealliance/wasmtime/blob/5f3b67ea055857020bd1ac1f2c3f7fa2e6c31ec0/crates/wasmtime/src/runtime/component/instance.rs#L424 + // - https://github.com/bytecodealliance/wasmtime/blob/9c49989a2e71382fd8639288088472f150f2f534/crates/wasmtime/src/runtime/vm/component.rs#L847 } /// Ensure a component-local `error-context` table has been created @@ -1078,34 +1067,46 @@ impl<'a> Instantiator<'a, '_> { .bindgen .intrinsic(Intrinsic::Resource(ResourceIntrinsic::ResourceTableRemove)); + // Create the relevant handle table let rtid = resource_table_idx.as_u32(); if is_imported { + // imported uwriteln!( self.src.js, - "const handleTable{rtid} = [{rsc_table_flag}, 0];", + r#" + const handleTable{rtid} = [{rsc_table_flag}, 0]; + handleTable{rtid}._createdReps = new Set(); + "#, ); if !self.resources_initialized.contains_key(&resource_idx) { let ridx = resource_idx.as_u32(); uwriteln!( self.src.js, - "const captureTable{ridx} = new Map(); - let captureCnt{ridx} = 0;" + r#" + const captureTable{ridx} = new Map(); + let captureCnt{ridx} = 0; + "# ); self.resources_initialized.insert(resource_idx, true); } } else { + // non imported let finalization_registry_create = self .bindgen .intrinsic(Intrinsic::FinalizationRegistryCreate); uwriteln!( self.src.js, - "const handleTable{rtid} = [{rsc_table_flag}, 0]; - const finalizationRegistry{rtid} = {finalization_registry_create}((handle) => {{ - const {{ rep }} = {rsc_table_remove}(handleTable{rtid}, handle);{maybe_dtor} - }}); - ", + r#" + const handleTable{rtid} = [{rsc_table_flag}, 0]; + handleTable{rtid}._createdReps = new Set(); + const finalizationRegistry{rtid} = {finalization_registry_create}((handle) => {{ + const {{ rep }} = {rsc_table_remove}(handleTable{rtid}, handle);{maybe_dtor} + }}); + "#, ); } + + // Add the handle table to the global list uwriteln!(self.src.js, "{handle_tables}[{rtid}] = handleTable{rtid};"); self.resource_tables_initialized .insert(resource_table_idx, true);