Callable: use Cow<str> instead of AsArg<GString>

Provided arguments are mostly string literals, which are cheap to store in `Cow`, but
expensive to convert to `GString`. Doing this on every call can have quite an impact.

Author: Bromeon

godot-core/src/builtin/callable.rs

@@ -11,7 +11,7 @@ use std::{fmt, ptr};
 use godot_ffi as sys;
 use sys::{ffi_methods, ExtVariantType, GodotFfi};
 
-use crate::builtin::{inner, AnyArray, GString, StringName, Variant};
+use crate::builtin::{inner, AnyArray, CowStr, StringName, Variant};
 use crate::meta::{GodotType, ToGodot};
 use crate::obj::bounds::DynMemory;
 use crate::obj::{Bounds, Gd, GodotClass, InstanceId, Singleton};
@@ -114,10 +114,10 @@ impl Callable {
         };
 
         #[cfg(feature = "experimental-threads")]
-        let callable = Self::from_sync_fn(&callable_name, function);
+        let callable = Self::from_sync_fn(callable_name, function);
 
         #[cfg(not(feature = "experimental-threads"))]
-        let callable = Self::from_fn(&callable_name, function);
+        let callable = Self::from_fn(callable_name, function);
 
         callable
     }
@@ -161,9 +161,14 @@ impl Callable {
     where
         R: ToGodot,
         F: 'static + FnMut(&[&Variant]) -> R,
-        S: meta::AsArg<GString>,
+        S: Into<CowStr>,
     {
-        Self::from_fn_wrapper(name, rust_function, Some(std::thread::current().id()), None)
+        Self::from_fn_wrapper(
+            name.into(),
+            rust_function,
+            Some(std::thread::current().id()),
+            None,
+        )
     }
 
     /// Creates a new callable linked to the given object from **single-threaded** Rust function or closure.
@@ -179,10 +184,10 @@ impl Callable {
         R: ToGodot,
         T: GodotClass,
         F: 'static + FnMut(&[&Variant]) -> R,
-        S: meta::AsArg<GString>,
+        S: Into<CowStr>,
     {
         Self::from_fn_wrapper(
-            name,
+            name.into(),
             rust_function,
             Some(std::thread::current().id()),
             Some(linked_object.instance_id()),
@@ -196,14 +201,19 @@ impl Callable {
     pub fn from_local_fn<F, S>(name: S, mut rust_function: F) -> Self
     where
         F: 'static + FnMut(&[&Variant]) -> Result<Variant, ()>,
-        S: meta::AsArg<GString>,
+        S: Into<CowStr>,
     {
-        meta::arg_into_owned!(name);
+        let name_cow = name.into();
 
-        Self::from_fn(&name, move |args| {
-            // Ignore errors.
-            rust_function(args).unwrap_or_else(|()| Variant::nil())
-        })
+        Self::from_fn_wrapper(
+            name_cow,
+            move |args| {
+                // Ignore errors.
+                rust_function(args).unwrap_or_else(|()| Variant::nil())
+            },
+            Some(std::thread::current().id()),
+            None,
+        )
     }
 
     /// Create callable from **single-threaded** Rust function or closure that can only be called once.
@@ -216,38 +226,47 @@ impl Callable {
     where
         R: ToGodot,
         F: 'static + FnOnce(&[&Variant]) -> R,
-        S: meta::AsArg<GString>,
+        S: Into<CowStr>,
     {
-        meta::arg_into_owned!(name);
+        let name_cow = name.into();
 
         let mut rust_fn_once = Some(rust_function);
-        Self::from_fn(&name, move |args| {
-            let rust_fn_once = rust_fn_once
-                .take()
-                .expect("callable created with from_once_fn() has already been consumed");
-
-            rust_fn_once(args)
-        })
+        Self::from_fn_wrapper(
+            name_cow,
+            move |args| {
+                let rust_fn_once = rust_fn_once
+                    .take()
+                    .expect("callable created with from_once_fn() has already been consumed");
+
+                rust_fn_once(args)
+            },
+            Some(std::thread::current().id()),
+            None,
+        )
     }
 
     #[cfg(feature = "trace")] // Test only.
     #[doc(hidden)]
     pub fn __once_fn<F, S>(name: S, rust_function: F) -> Self
     where
         F: 'static + FnOnce(&[&Variant]) -> Variant,
-        S: meta::AsArg<GString>,
+        S: Into<CowStr>,
     {
         Self::from_once_fn(name, rust_function)
     }
 
     pub(crate) fn with_scoped_fn<S, F, Fc, R>(name: S, rust_function: F, callable_usage: Fc) -> R
     where
-        S: meta::AsArg<GString>,
+        S: Into<CowStr>,
         F: FnMut(&[&Variant]) -> Variant,
         Fc: FnOnce(&Callable) -> R,
     {
-        let callable =
-            Self::from_fn_wrapper(name, rust_function, Some(std::thread::current().id()), None);
+        let callable = Self::from_fn_wrapper(
+            name.into(),
+            rust_function,
+            Some(std::thread::current().id()),
+            None,
+        );
 
         callable_usage(&callable)
     }
@@ -276,9 +295,9 @@ impl Callable {
     where
         R: ToGodot,
         F: 'static + Send + Sync + FnMut(&[&Variant]) -> R,
-        S: meta::AsArg<GString>,
+        S: Into<CowStr>,
     {
-        Self::from_fn_wrapper(name, rust_function, None, None)
+        Self::from_fn_wrapper(name.into(), rust_function, None, None)
     }
 
     /// Create a highly configurable callable from Rust.
@@ -305,21 +324,19 @@ impl Callable {
         Self::from_custom_info(info)
     }
 
-    fn from_fn_wrapper<F, R, S>(
-        _name: S,
+    fn from_fn_wrapper<F, R>(
+        name: CowStr,
         rust_function: F,
         thread_id: Option<ThreadId>,
         linked_object_id: Option<InstanceId>,
     ) -> Self
     where
         F: FnMut(&[&Variant]) -> R,
         R: ToGodot,
-        S: meta::AsArg<GString>,
     {
         let wrapper = FnWrapper {
             rust_function,
-            #[cfg(safeguards_balanced)]
-            name: { _name.into_arg().cow_into_owned() },
+            name,
             thread_id,
             linked_object_id,
         };
@@ -332,7 +349,6 @@ impl Callable {
             callable_userdata: Box::into_raw(Box::new(userdata)) as *mut std::ffi::c_void,
             call_func: Some(rust_callable_call_fn::<F, R>),
             free_func: Some(rust_callable_destroy::<FnWrapper<F>>),
-            #[cfg(safeguards_balanced)]
             to_string_func: Some(rust_callable_to_string_named::<F>),
             is_valid_func: Some(rust_callable_is_valid),
             ..Self::default_callable_custom_info()
@@ -601,8 +617,7 @@ mod custom_callable {
 
     pub(crate) struct FnWrapper<F> {
         pub(super) rust_function: F,
-        #[cfg(safeguards_balanced)]
-        pub(super) name: GString,
+        pub(super) name: CowStr,
 
         /// `None` if the callable is multi-threaded ([`Callable::from_sync_fn`]).
         pub(super) thread_id: Option<ThreadId>,
@@ -692,24 +707,20 @@ mod custom_callable {
     {
         let arg_refs: &[&Variant] = Variant::borrow_ref_slice(p_args, p_argument_count as usize);
 
-        let name = name_or_optimized! {
-            let w: &FnWrapper<F> = CallableUserdata::inner_from_raw(callable_userdata);
-            w.name.to_string()
-        };
-        let ctx = meta::CallContext::custom_callable(&name);
+        let w: &FnWrapper<F> = CallableUserdata::inner_from_raw(callable_userdata);
+        let ctx = meta::CallContext::custom_callable(&w.name);
 
         crate::private::handle_fallible_varcall(&ctx, &mut *r_error, move || {
             // Get the FnWrapper again inside closure so the FnMut doesn't have to be UnwindSafe.
             let w: &mut FnWrapper<F> = CallableUserdata::inner_from_raw(callable_userdata);
 
-            let name = name_or_optimized!(w.name.to_string());
-
             // NOTE: this panic is currently not propagated to the caller, but results in an error message and Nil return.
             // See comments in itest callable_call() for details.
             sys::balanced_assert!(
                 w.thread_id.is_none() || w.thread_id == Some(std::thread::current().id()),
-                "Callable '{name}' created with from_fn() must be called from the same thread it was created in.\n\
-                If you need to call it from any thread, use from_sync_fn() instead (requires `experimental-threads` feature)."
+                "Callable '{}' created with from_fn() must be called from the same thread it was created in.\n\
+                If you need to call it from any thread, use from_sync_fn() instead (requires `experimental-threads` feature).",
+                w.name
             );
 
             let result = (w.rust_function)(arg_refs).to_variant();
@@ -754,15 +765,15 @@ mod custom_callable {
         *r_is_valid = sys::conv::SYS_TRUE;
     }
 
-    #[cfg(safeguards_balanced)]
     pub unsafe extern "C" fn rust_callable_to_string_named<F>(
         callable_userdata: *mut std::ffi::c_void,
         r_is_valid: *mut sys::GDExtensionBool,
         r_out: sys::GDExtensionStringPtr,
     ) {
         let w: &mut FnWrapper<F> = CallableUserdata::inner_from_raw(callable_userdata);
+        let gstring = GString::from(w.name.as_ref());
 
-        w.name.clone().move_into_string_ptr(r_out);
+        gstring.move_into_string_ptr(r_out);
         *r_is_valid = sys::conv::SYS_TRUE;
     }
 

godot-core/src/obj/base.rs

@@ -217,7 +217,7 @@ impl<T: GodotClass> Base<T> {
         });
 
         let name = format!("Base<{}> deferred unref", T::class_id());
-        let callable = Callable::from_once_fn(&name, move |_args| {
+        let callable = Callable::from_once_fn(name, move |_args| {
             Self::drop_strong_ref(instance_id);
         });
 

godot-core/src/obj/gd.rs

@@ -12,7 +12,7 @@ use godot_ffi as sys;
 use godot_ffi::is_main_thread;
 use sys::{static_assert_eq_size_align, SysPtr as _};
 
-use crate::builtin::{Callable, GString, NodePath, StringName, Variant};
+use crate::builtin::{Callable, CowStr, NodePath, StringName, Variant};
 use crate::meta::error::{ConvertError, FromFfiError};
 use crate::meta::{
     ArrayElement, AsArg, ClassId, FromGodot, GodotConvert, GodotType, PropertyHintInfo, RefArg,
@@ -586,7 +586,7 @@ impl<T: GodotClass> Gd<T> {
     /// If you need a Callable which can live indefinitely, use [`Callable::from_fn()`].
     pub fn linked_callable<R, F>(
         &self,
-        method_name: impl AsArg<GString>,
+        method_name: impl Into<CowStr>,
         rust_function: F,
     ) -> Callable
     where

godot-core/src/registry/signal/connect_builder.rs

@@ -8,7 +8,7 @@
 use super::{make_callable_name, make_godot_fn};
 #[cfg(feature = "experimental-threads")]
 use crate::builtin::Callable;
-use crate::builtin::{GString, Variant};
+use crate::builtin::{CowStr, Variant};
 use crate::classes::object::ConnectFlags;
 use crate::meta;
 use crate::meta::{InParamTuple, ObjectToOwned};
@@ -76,7 +76,7 @@ pub struct ConnectBuilder<'ts, 'c, C: WithSignals, Ps> {
 #[derive(Default)]
 struct BuilderData {
     /// User-specified name; if not provided, the Rust RTTI type name of the function is used.
-    callable_name: Option<GString>,
+    callable_name: Option<CowStr>,
 
     /// Godot connection flags.
     connect_flags: Option<ConnectFlags>,
@@ -98,14 +98,13 @@ where
     /// Name of the `Callable`, mostly used for debugging.
     ///
     /// If not provided, the Rust type name of the function/method is used.
-    pub fn name(mut self, name: impl meta::AsArg<GString>) -> Self {
+    pub fn name(mut self, name: impl Into<CowStr>) -> Self {
         assert!(
             self.data.callable_name.is_none(),
             "name() called twice on the same builder."
         );
 
-        meta::arg_into_owned!(name);
-        self.data.callable_name = Some(name);
+        self.data.callable_name = Some(name.into());
         self
     }
 
@@ -129,9 +128,9 @@ where
         godot_fn: impl FnMut(&[&Variant]) -> Variant + 'static,
         bound: &Gd<impl GodotClass>,
     ) -> ConnectHandle {
-        let callable_name = match &self.data.callable_name {
+        let callable_name = match self.data.callable_name {
             Some(user_provided_name) => user_provided_name,
-            None => &make_callable_name::<F>(),
+            None => make_callable_name::<F>().to_string().into(),
         };
 
         let callable = bound.linked_callable(callable_name, godot_fn);
@@ -311,9 +310,9 @@ impl<C: WithSignals, Ps: InParamTuple + 'static> ConnectBuilder<'_, '_, C, Ps> {
                 .call((), args);
         });
 
-        let callable_name = match &self.data.callable_name {
+        let callable_name = match self.data.callable_name {
             Some(user_provided_name) => user_provided_name,
-            None => &make_callable_name::<F>(),
+            None => make_callable_name::<F>(),
         };
 
         let callable = Callable::from_sync_fn(callable_name, godot_fn);

godot-core/src/registry/signal/mod.rs

@@ -11,12 +11,14 @@ mod signal_object;
 mod signal_receiver;
 mod typed_signal;
 
+use std::borrow::Cow;
+
 pub(crate) use connect_builder::*;
 pub(crate) use connect_handle::*;
 pub(crate) use signal_object::*;
 pub(crate) use typed_signal::*;
 
-use crate::builtin::{GString, Variant};
+use crate::builtin::{CowStr, Variant};
 use crate::meta;
 
 // Used in `godot` crate.
@@ -52,9 +54,9 @@ where
 }
 
 // Used by both `TypedSignal` and `ConnectBuilder`.
-fn make_callable_name<F>() -> GString {
+fn make_callable_name<F>() -> CowStr {
     // When using sys::short_type_name() in the future, make sure global "func" and member "MyClass::func" are rendered as such.
     // PascalCase heuristic should then be good enough.
 
-    std::any::type_name::<F>().into()
+    Cow::Borrowed(std::any::type_name::<F>())
 }

godot-core/src/registry/signal/typed_signal.rs

@@ -132,7 +132,7 @@ impl<'c, C: WithSignals, Ps: meta::ParamTuple> TypedSignal<'c, C, Ps> {
         bound: &Gd<impl GodotClass>,
     ) -> ConnectHandle {
         let callable_name = make_callable_name::<F>();
-        let callable = bound.linked_callable(&callable_name, godot_fn);
+        let callable = bound.linked_callable(callable_name, godot_fn);
         self.inner_connect_untyped(callable, None)
     }
 

itest/rust/src/benchmarks/mod.rs

@@ -117,15 +117,17 @@ fn packed_array_from_iter_unknown_size() -> PackedInt32Array {
 #[bench(manual)]
 fn call_callv_rust_fn() -> BenchResult {
     let callable = Callable::from_fn("RustFunction", |_| ());
+    let arg = varray![];
 
-    bench_measure(25, || callable.callv(&varray![]))
+    bench_measure(25000, || callable.callv(&arg))
 }
 
 #[bench(manual)]
 fn call_callv_custom() -> BenchResult {
     let callable = Callable::from_custom(MyRustCallable {});
+    let arg = varray![];
 
-    bench_measure(25, || callable.callv(&varray![]))
+    bench_measure(25, || callable.callv(&arg))
 }
 
 // ----------------------------------------------------------------------------------------------------------------------------------------------