From 0aae0739e842e98b9c9f92f91794bd907e7be635 Mon Sep 17 00:00:00 2001 From: James Sturtevant Date: Fri, 10 Jan 2025 11:40:50 -0800 Subject: [PATCH 1/3] When calling an import we don't need to copy data to a new structure for cannoncal types. This avoid the extra copy of data in this scenario Signed-off-by: James Sturtevant --- crates/csharp/src/function.rs | 27 ++++++++++++++------------- 1 file changed, 14 insertions(+), 13 deletions(-) diff --git a/crates/csharp/src/function.rs b/crates/csharp/src/function.rs index faa6aad35..4f25d9118 100644 --- a/crates/csharp/src/function.rs +++ b/crates/csharp/src/function.rs @@ -581,21 +581,22 @@ impl Bindgen for FunctionBindgen<'_, '_> { } Instruction::ListCanonLower { element, realloc } => { - let list = &operands[0]; + let list: &String = &operands[0]; let (_size, ty) = list_element_info(element); match self.interface_gen.direction { Direction::Import => { - let buffer: String = self.locals.tmp("buffer"); + let buffer: String = self.locals.tmp("bufferPtr"); uwrite!( self.src, " - void* {buffer} = stackalloc {ty}[({list}).Length]; - {list}.AsSpan<{ty}>().CopyTo(new Span<{ty}>({buffer}, {list}.Length)); + fixed ({ty}* {buffer} = new ReadOnlyMemory<{ty}>({list}).Span) + {{ " ); - results.push(format!("(int){buffer}")); + results.push(format!("(nint){buffer}")); results.push(format!("({list}).Length")); + self.fixed = self.fixed + 1; } Direction::Export => { let address = self.locals.tmp("address"); @@ -973,11 +974,11 @@ impl Bindgen for FunctionBindgen<'_, '_> { uwriteln!(self.src, "return ({results});") } } + } - // Close all the fixed blocks. - for _ in 0..self.fixed { - uwriteln!(self.src, "}}"); - } + // Close all the fixed blocks. + for _ in 0..self.fixed { + uwriteln!(self.src, "}}"); } } @@ -1063,7 +1064,7 @@ impl Bindgen for FunctionBindgen<'_, '_> { match direction { Direction::Import => { - let import_name = self.interface_gen.type_name_with_qualifier(&Type::Id(id), true); + let import_name = self.interface_gen.type_name_with_qualifier(&Type::Id(id), true); if let FunctionKind::Constructor(_) = self.kind { resource = "this".to_owned(); @@ -1082,13 +1083,13 @@ impl Bindgen for FunctionBindgen<'_, '_> { Direction::Export => { self.interface_gen.csharp_gen.needs_rep_table = true; - let export_name = self.interface_gen.csharp_gen.all_resources[&id].export_impl_name(); + let export_name = self.interface_gen.csharp_gen.all_resources[&id].export_impl_name(); if is_own { uwriteln!( self.src, "var {resource} = ({export_name}) {export_name}.repTable.Get\ - ({export_name}.WasmInterop.wasmImportResourceRep({op})); - {resource}.Handle = {op};" + ({export_name}.WasmInterop.wasmImportResourceRep({op})); + {resource}.Handle = {op};" ); } else { uwriteln!(self.src, "var {resource} = ({export_name}) {export_name}.repTable.Get({op});"); From 9aa44082b5c787aef2936f335a2a2d1f4c6a7a2a Mon Sep 17 00:00:00 2001 From: James Sturtevant Date: Wed, 15 Jan 2025 10:36:27 -0800 Subject: [PATCH 2/3] Use a pinned gc handle to get a pointer to the list Using a span and fixed keyword won't work with variants due to the fact that the external import call requires different types. Nesting of the fixed commands also become unwiedly Signed-off-by: James Sturtevant --- crates/csharp/src/function.rs | 25 +++++++++++++------------ 1 file changed, 13 insertions(+), 12 deletions(-) diff --git a/crates/csharp/src/function.rs b/crates/csharp/src/function.rs index 4f25d9118..8f04badc2 100644 --- a/crates/csharp/src/function.rs +++ b/crates/csharp/src/function.rs @@ -582,21 +582,23 @@ impl Bindgen for FunctionBindgen<'_, '_> { Instruction::ListCanonLower { element, realloc } => { let list: &String = &operands[0]; - let (_size, ty) = list_element_info(element); - match self.interface_gen.direction { Direction::Import => { - let buffer: String = self.locals.tmp("bufferPtr"); + let ptr: String = self.locals.tmp("listPtr"); + let handle: String = self.locals.tmp("gcHandle"); + // Dispite the name GCHandle.Alloc here this does not actually allocate memory on the heap. + // It pins the array with the garbage collector so that it can be passed to unmanaged code + // It is required to free the pin after use which is done in the Cleanup section uwrite!( self.src, " - fixed ({ty}* {buffer} = new ReadOnlyMemory<{ty}>({list}).Span) - {{ + var {handle} = GCHandle.Alloc({list}, GCHandleType.Pinned); + var {ptr} = {handle}.AddrOfPinnedObject(); " ); - results.push(format!("(nint){buffer}")); + results.push(format!("{ptr}")); results.push(format!("({list}).Length")); - self.fixed = self.fixed + 1; + self.cleanup.push(Cleanup { address: handle }); } Direction::Export => { let address = self.locals.tmp("address"); @@ -974,11 +976,10 @@ impl Bindgen for FunctionBindgen<'_, '_> { uwriteln!(self.src, "return ({results});") } } - } - - // Close all the fixed blocks. - for _ in 0..self.fixed { - uwriteln!(self.src, "}}"); + // Close all the fixed blocks. + for _ in 0..self.fixed { + uwriteln!(self.src, "}}"); + } } } From 9f3265203992f36c937dc3654a8892d04fea2a73 Mon Sep 17 00:00:00 2001 From: James Sturtevant Date: Wed, 15 Jan 2025 13:55:11 -0800 Subject: [PATCH 3/3] Apply suggestions from code review Co-authored-by: Joel Dice --- crates/csharp/src/function.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/crates/csharp/src/function.rs b/crates/csharp/src/function.rs index 8f04badc2..766fa929f 100644 --- a/crates/csharp/src/function.rs +++ b/crates/csharp/src/function.rs @@ -586,9 +586,9 @@ impl Bindgen for FunctionBindgen<'_, '_> { Direction::Import => { let ptr: String = self.locals.tmp("listPtr"); let handle: String = self.locals.tmp("gcHandle"); - // Dispite the name GCHandle.Alloc here this does not actually allocate memory on the heap. - // It pins the array with the garbage collector so that it can be passed to unmanaged code - // It is required to free the pin after use which is done in the Cleanup section + // Despite the name GCHandle.Alloc here this does not actually allocate memory on the heap. + // It pins the array with the garbage collector so that it can be passed to unmanaged code. + // It is required to free the pin after use which is done in the Cleanup section. uwrite!( self.src, "