From 0d4b2b7199e100b9838f4bd69a0ec73079987f6f Mon Sep 17 00:00:00 2001 From: Fredrik Andersson Date: Wed, 13 Dec 2023 11:53:18 +0100 Subject: [PATCH 1/7] Reenable GC for references, hold values over async points --- crates/cli-support/src/js/binding.rs | 29 +++++++++++++++++++++++++--- 1 file changed, 26 insertions(+), 3 deletions(-) diff --git a/crates/cli-support/src/js/binding.rs b/crates/cli-support/src/js/binding.rs index 76b61b42f86..a2de2c7641e 100644 --- a/crates/cli-support/src/js/binding.rs +++ b/crates/cli-support/src/js/binding.rs @@ -171,6 +171,7 @@ impl<'a, 'b> Builder<'a, 'b> { &instr.instr, &mut self.log_error, &self.constructor, + asyncness )?; } @@ -565,6 +566,7 @@ fn instruction( instr: &Instruction, log_error: &mut bool, constructor: &Option, + asyncness: bool ) -> Result<(), Error> { match instr { Instruction::ArgGet(n) => { @@ -609,7 +611,7 @@ fn instruction( } // Call the function through an export of the underlying module. - let call = invoc.invoke(js.cx, &args, &mut js.prelude, log_error)?; + let call = invoc.invoke(js.cx, &args, &mut js.prelude, log_error, asyncness)?; // And then figure out how to actually handle where the call // happens. This is pretty conditional depending on the number of @@ -1019,7 +1021,8 @@ fn instruction( match constructor { Some(name) if name == class => { js.prelude(&format!("this.__wbg_ptr = {} >>> 0;", val)); - js.push(String::from("this")); + js.prelude(&format!("{}Finalization.register(this, this.__wbg_ptr, this);", class)); + js.push( "this".into()); } Some(_) | None => { js.cx.require_class_wrap(class); @@ -1282,11 +1285,31 @@ impl Invocation { args: &[String], prelude: &mut String, log_error: &mut bool, + asyncness: bool ) -> Result { match self { Invocation::Core { id, .. } => { + let name = cx.export_name_of(*id); - Ok(format!("wasm.{}({})", name, args.join(", "))) + if asyncness { + let mut argscopy = args.to_vec(); + argscopy[0] = String::from("that.__wbg_ptr"); + let root_objects: Vec<&str> = argscopy.iter().map(|arg: &String | { + arg.strip_suffix(".__wbg_ptr").unwrap_or(arg) + }).collect(); + prelude.push_str("let that = this;\n"); + Ok(format!(r#" + (function () {{ + return new Promise((resolve, reject) => {{ + that.__paramRefs = that._paramRefs || {{}}; + that.__paramRefs[that.__wbg_ptr>>>0] = [{}]; + wasm.{}({}).then(resolve).catch(reject).finally(() => {{ + delete that.__paramRefs[that.__wbg_ptr>>>0]; + }}); + }})}})()"#, root_objects.join(","), name, argscopy.join(", "))) + } else { + Ok(format!("wasm.{}({})", name, args.join(", "))) + } } Invocation::Adapter(id) => { let adapter = &cx.wit.adapters[id]; From 03c393ca699c937fedb1cb9878f19388f9d3e881 Mon Sep 17 00:00:00 2001 From: Fredrik Andersson Date: Wed, 13 Dec 2023 12:07:50 +0100 Subject: [PATCH 2/7] rustfmt --- crates/cli-support/src/js/binding.rs | 30 ++++++++++++++++++---------- 1 file changed, 19 insertions(+), 11 deletions(-) diff --git a/crates/cli-support/src/js/binding.rs b/crates/cli-support/src/js/binding.rs index a2de2c7641e..f94a88fc097 100644 --- a/crates/cli-support/src/js/binding.rs +++ b/crates/cli-support/src/js/binding.rs @@ -171,7 +171,7 @@ impl<'a, 'b> Builder<'a, 'b> { &instr.instr, &mut self.log_error, &self.constructor, - asyncness + asyncness, )?; } @@ -566,7 +566,7 @@ fn instruction( instr: &Instruction, log_error: &mut bool, constructor: &Option, - asyncness: bool + asyncness: bool, ) -> Result<(), Error> { match instr { Instruction::ArgGet(n) => { @@ -1021,8 +1021,11 @@ fn instruction( match constructor { Some(name) if name == class => { js.prelude(&format!("this.__wbg_ptr = {} >>> 0;", val)); - js.prelude(&format!("{}Finalization.register(this, this.__wbg_ptr, this);", class)); - js.push( "this".into()); + js.prelude(&format!( + "{}Finalization.register(this, this.__wbg_ptr, this);", + class + )); + js.push("this".into()); } Some(_) | None => { js.cx.require_class_wrap(class); @@ -1285,20 +1288,21 @@ impl Invocation { args: &[String], prelude: &mut String, log_error: &mut bool, - asyncness: bool + asyncness: bool, ) -> Result { match self { Invocation::Core { id, .. } => { - let name = cx.export_name_of(*id); if asyncness { let mut argscopy = args.to_vec(); argscopy[0] = String::from("that.__wbg_ptr"); - let root_objects: Vec<&str> = argscopy.iter().map(|arg: &String | { - arg.strip_suffix(".__wbg_ptr").unwrap_or(arg) - }).collect(); + let root_objects: Vec<&str> = argscopy + .iter() + .map(|arg: &String| arg.strip_suffix(".__wbg_ptr").unwrap_or(arg)) + .collect(); prelude.push_str("let that = this;\n"); - Ok(format!(r#" + Ok(format!( + r#" (function () {{ return new Promise((resolve, reject) => {{ that.__paramRefs = that._paramRefs || {{}}; @@ -1306,7 +1310,11 @@ impl Invocation { wasm.{}({}).then(resolve).catch(reject).finally(() => {{ delete that.__paramRefs[that.__wbg_ptr>>>0]; }}); - }})}})()"#, root_objects.join(","), name, argscopy.join(", "))) + }})}})()"#, + root_objects.join(","), + name, + argscopy.join(", ") + )) } else { Ok(format!("wasm.{}({})", name, args.join(", "))) } From 7f40be6b655aa1c4610fbf066b55d2a73a81e286 Mon Sep 17 00:00:00 2001 From: Fredrik Andersson Date: Wed, 13 Dec 2023 12:15:58 +0100 Subject: [PATCH 3/7] Only generate glue if there are args --- crates/cli-support/src/js/binding.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/cli-support/src/js/binding.rs b/crates/cli-support/src/js/binding.rs index f94a88fc097..79a3be56bad 100644 --- a/crates/cli-support/src/js/binding.rs +++ b/crates/cli-support/src/js/binding.rs @@ -1293,7 +1293,7 @@ impl Invocation { match self { Invocation::Core { id, .. } => { let name = cx.export_name_of(*id); - if asyncness { + if asyncness && args.len() > 0 { let mut argscopy = args.to_vec(); argscopy[0] = String::from("that.__wbg_ptr"); let root_objects: Vec<&str> = argscopy From 4c2b39146d923bc93145970f1ee7041e901dc54e Mon Sep 17 00:00:00 2001 From: Fredrik Andersson Date: Wed, 13 Dec 2023 12:27:54 +0100 Subject: [PATCH 4/7] Only generate if weakrefs are enabled --- crates/cli-support/src/js/binding.rs | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/crates/cli-support/src/js/binding.rs b/crates/cli-support/src/js/binding.rs index 79a3be56bad..8b72774b5d1 100644 --- a/crates/cli-support/src/js/binding.rs +++ b/crates/cli-support/src/js/binding.rs @@ -1021,10 +1021,12 @@ fn instruction( match constructor { Some(name) if name == class => { js.prelude(&format!("this.__wbg_ptr = {} >>> 0;", val)); - js.prelude(&format!( - "{}Finalization.register(this, this.__wbg_ptr, this);", - class - )); + if js.cx.config.weak_refs { + js.prelude(&format!( + "{}Finalization.register(this, this.__wbg_ptr, this);", + class + )); + } js.push("this".into()); } Some(_) | None => { @@ -1293,7 +1295,7 @@ impl Invocation { match self { Invocation::Core { id, .. } => { let name = cx.export_name_of(*id); - if asyncness && args.len() > 0 { + if asyncness && args.len() > 0 && cx.config.weak_refs { let mut argscopy = args.to_vec(); argscopy[0] = String::from("that.__wbg_ptr"); let root_objects: Vec<&str> = argscopy From 7fb9fb17abb36d2ea7444c1769125f5dbfa3195b Mon Sep 17 00:00:00 2001 From: Fredrik Andersson Date: Wed, 13 Dec 2023 12:57:55 +0100 Subject: [PATCH 5/7] clippy fix --- crates/cli-support/src/js/binding.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/cli-support/src/js/binding.rs b/crates/cli-support/src/js/binding.rs index 8b72774b5d1..f216efe1cd6 100644 --- a/crates/cli-support/src/js/binding.rs +++ b/crates/cli-support/src/js/binding.rs @@ -1295,7 +1295,7 @@ impl Invocation { match self { Invocation::Core { id, .. } => { let name = cx.export_name_of(*id); - if asyncness && args.len() > 0 && cx.config.weak_refs { + if asyncness && !args.is_empty() && cx.config.weak_refs { let mut argscopy = args.to_vec(); argscopy[0] = String::from("that.__wbg_ptr"); let root_objects: Vec<&str> = argscopy From 1de7e70a0dfd84e47daf17df5b91f754f0c8296e Mon Sep 17 00:00:00 2001 From: Fredrik Andersson Date: Thu, 14 Dec 2023 11:24:05 +0100 Subject: [PATCH 6/7] Fixed typo and removed unnecessary uint conversion --- crates/cli-support/src/js/binding.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/crates/cli-support/src/js/binding.rs b/crates/cli-support/src/js/binding.rs index f216efe1cd6..478f3c5c439 100644 --- a/crates/cli-support/src/js/binding.rs +++ b/crates/cli-support/src/js/binding.rs @@ -1307,10 +1307,10 @@ impl Invocation { r#" (function () {{ return new Promise((resolve, reject) => {{ - that.__paramRefs = that._paramRefs || {{}}; - that.__paramRefs[that.__wbg_ptr>>>0] = [{}]; + that.__paramRefs = that.__paramRefs || {{}}; + that.__paramRefs[that.__wbg_ptr] = [{}]; wasm.{}({}).then(resolve).catch(reject).finally(() => {{ - delete that.__paramRefs[that.__wbg_ptr>>>0]; + delete that.__paramRefs[that.__wbg_ptr]; }}); }})}})()"#, root_objects.join(","), From 6d990736d1aafa73e86c207ee37e82c8fbd2c661 Mon Sep 17 00:00:00 2001 From: Fredrik Andersson Date: Thu, 14 Dec 2023 12:29:27 +0100 Subject: [PATCH 7/7] Don't keep references to functions --- crates/cli-support/src/js/binding.rs | 43 +++++++++++++++++++++------- 1 file changed, 32 insertions(+), 11 deletions(-) diff --git a/crates/cli-support/src/js/binding.rs b/crates/cli-support/src/js/binding.rs index 478f3c5c439..25614d32194 100644 --- a/crates/cli-support/src/js/binding.rs +++ b/crates/cli-support/src/js/binding.rs @@ -8,6 +8,7 @@ use crate::js::Context; use crate::wit::InstructionData; use crate::wit::{Adapter, AdapterId, AdapterKind, AdapterType, Instruction}; use anyhow::{anyhow, bail, Error}; +use std::collections::HashSet; use std::fmt::Write; use walrus::{Module, ValType}; @@ -1284,6 +1285,20 @@ impl Invocation { } } + fn transform_this_to_that_args(&self, args: &[String]) -> Vec { + args.iter() + .map(|arg| arg.replace("this.", "that.")) + .collect() + } + fn is_non_function_call(&self, obj: &str) -> bool { + !obj.contains('(') + } + fn find_root_objects(&self, args: &[String]) -> Vec { + args.into_iter() + .map(|arg| arg.split('.').next().unwrap().to_string()) + .filter(|val| self.is_non_function_call(val)) + .collect() + } fn invoke( &self, cx: &mut Context, @@ -1296,27 +1311,33 @@ impl Invocation { Invocation::Core { id, .. } => { let name = cx.export_name_of(*id); if asyncness && !args.is_empty() && cx.config.weak_refs { - let mut argscopy = args.to_vec(); - argscopy[0] = String::from("that.__wbg_ptr"); - let root_objects: Vec<&str> = argscopy - .iter() - .map(|arg: &String| arg.strip_suffix(".__wbg_ptr").unwrap_or(arg)) + let transformed_args = self.transform_this_to_that_args(args); + let roots_objects = self.find_root_objects(&transformed_args); + + let unique_roots: Vec = roots_objects + .into_iter() + .collect::>() + .into_iter() .collect(); prelude.push_str("let that = this;\n"); - Ok(format!( + let wrapper = format!( r#" (function () {{ return new Promise((resolve, reject) => {{ - that.__paramRefs = that.__paramRefs || {{}}; - that.__paramRefs[that.__wbg_ptr] = [{}]; + let uniqueRoots = [{}]; + if(uniqueRoots.length) {{ + that.__paramRefs = that.__paramRefs || {{}}; + that.__paramRefs[that.__wbg_ptr] = unique_roots; + }} wasm.{}({}).then(resolve).catch(reject).finally(() => {{ delete that.__paramRefs[that.__wbg_ptr]; }}); }})}})()"#, - root_objects.join(","), + unique_roots.join(","), name, - argscopy.join(", ") - )) + transformed_args.join(", ") + ); + Ok(wrapper) } else { Ok(format!("wasm.{}({})", name, args.join(", "))) }