Merge pull request #1314 from godot-rust/perf/object-arg-by-ref

`AsArg` for objects now consistently passes by reference

Author: Bromeon

godot-core/src/meta/args/as_arg.rs

@@ -91,18 +91,29 @@ where
     // ergonomics out weigh the runtime cost. Using the CowArg allows us to create a blanket implementation of the trait for all types that
     // implement ToGodot.
     #[doc(hidden)]
-    fn into_arg<'r>(self) -> CowArg<'r, T>
+    fn into_arg<'arg>(self) -> CowArg<'arg, T>
     where
-        Self: 'r;
+        Self: 'arg;
+
+    /// FFI-optimized argument conversion that may use `FfiObject` when beneficial.
+    ///
+    /// Defaults to calling `into_arg()`, which always works, but might be an `Owned` for a conservative approach (e.g. object upcast).
+    #[doc(hidden)]
+    fn into_ffi_arg<'arg>(self) -> CowArg<'arg, T>
+    where
+        Self: 'arg,
+    {
+        self.into_arg()
+    }
 }
 
 impl<T> AsArg<T> for &T
 where
     T: ToGodot<Pass = ByRef>,
 {
-    fn into_arg<'r>(self) -> CowArg<'r, T>
+    fn into_arg<'arg>(self) -> CowArg<'arg, T>
     where
-        Self: 'r,
+        Self: 'arg,
     {
         CowArg::Borrowed(self)
     }
@@ -112,9 +123,9 @@ impl<T> AsArg<T> for T
 where
     T: ToGodot<Pass = ByValue>,
 {
-    fn into_arg<'r>(self) -> CowArg<'r, T>
+    fn into_arg<'arg>(self) -> CowArg<'arg, T>
     where
-        Self: 'r,
+        Self: 'arg,
     {
         CowArg::Owned(self)
     }
@@ -125,98 +136,169 @@ where
 
 // TODO(v0.4): all objects + optional objects should be pass-by-ref.
 
+// Convert `Gd` -> `Gd` (with upcast).
 impl<T, Base> AsArg<Gd<Base>> for &Gd<T>
 where
     T: Inherits<Base>,
     Base: GodotClass,
 {
-    fn into_arg<'r>(self) -> CowArg<'r, Gd<Base>>
+    fn into_arg<'arg>(self) -> CowArg<'arg, Gd<Base>>
     where
-        Self: 'r,
+        Self: 'arg,
     {
-        CowArg::Owned(self.clone().upcast::<Base>())
+        if T::IS_SAME_CLASS {
+            // SAFETY: T == Base, so &Gd<T> can be treated as &Gd<Base>.
+            let gd_ref = unsafe { std::mem::transmute::<&Gd<T>, &Gd<Base>>(self) };
+            CowArg::Borrowed(gd_ref)
+        } else {
+            // Different types: clone and upcast. May incur ref-count increment for RefCounted objects, but the common path
+            // of FFI passing is already optimized.
+            CowArg::Owned(self.clone().upcast())
+        }
+    }
+
+    fn into_ffi_arg<'arg>(self) -> CowArg<'arg, Gd<Base>>
+    where
+        Self: 'arg,
+    {
+        // SAFETY: ObjectArg exists only during FFI call.
+        let arg = unsafe { ObjectArg::from_gd(self) };
+        CowArg::FfiObject(arg)
     }
 }
 
-impl<T, U, D> AsArg<DynGd<T, D>> for &DynGd<U, D>
+/// Convert `DynGd` -> `DynGd` (with upcast).
+impl<T, D, Base> AsArg<DynGd<Base, D>> for &DynGd<T, D>
 where
-    T: GodotClass,
-    U: Inherits<T>,
+    T: Inherits<Base>,
     D: ?Sized,
+    Base: GodotClass,
 {
-    fn into_arg<'r>(self) -> CowArg<'r, DynGd<T, D>>
+    //noinspection RsConstantConditionIf - false positive in IDE for `T::IS_SAME_CLASS`.
+    fn into_arg<'arg>(self) -> CowArg<'arg, DynGd<Base, D>>
     where
-        Self: 'r,
+        Self: 'arg,
     {
-        CowArg::Owned(self.clone().upcast::<T>())
+        if T::IS_SAME_CLASS {
+            // SAFETY: T == Base, so &DynGd<T, D> can be treated as &DynGd<Base, D>.
+            let gd_ref = unsafe { std::mem::transmute::<&Gd<T>, &DynGd<Base, D>>(self) };
+            CowArg::Borrowed(gd_ref)
+        } else {
+            // Different types: clone and upcast. May incur ref-count increment for RefCounted objects, but the common path
+            // of FFI passing is already optimized.
+            CowArg::Owned(self.clone().upcast())
+        }
+    }
+
+    fn into_ffi_arg<'arg>(self) -> CowArg<'arg, DynGd<Base, D>>
+    where
+        Self: 'arg,
+    {
+        // SAFETY: ObjectArg exists only during FFI call.
+        let arg = unsafe { ObjectArg::from_gd(self) };
+        CowArg::FfiObject(arg)
     }
 }
 
-// Convert DynGd -> Gd (with upcast).
-impl<'r, T, U, D> AsArg<Gd<T>> for &'r DynGd<U, D>
+// Convert `DynGd` -> `Gd` (with upcast).
+impl<T, D, Base> AsArg<Gd<Base>> for &DynGd<T, D>
 where
-    T: GodotClass,
-    U: Inherits<T>,
+    T: Inherits<Base>,
     D: ?Sized,
+    Base: GodotClass,
 {
-    fn into_arg<'cow>(self) -> CowArg<'cow, Gd<T>>
+    fn into_arg<'arg>(self) -> CowArg<'arg, Gd<Base>>
+    where
+        Self: 'arg,
+    {
+        let gd_ref: &Gd<T> = self; // DynGd -> Gd deref.
+        AsArg::into_arg(gd_ref)
+    }
+
+    fn into_ffi_arg<'arg>(self) -> CowArg<'arg, Gd<Base>>
     where
-        'r: 'cow,
+        Self: 'arg,
     {
-        CowArg::Owned(self.clone().upcast::<T>().into_gd())
+        let gd_ref: &Gd<T> = self; // DynGd -> Gd deref.
+        AsArg::into_ffi_arg(gd_ref)
     }
 }
 
 // ----------------------------------------------------------------------------------------------------------------------------------------------
 // Optional object (Gd + DynGd) impls
 
-impl<T, U> AsArg<Option<Gd<T>>> for &Option<Gd<U>>
+/// Convert `Option<&Gd>` -> `Option<Gd>` (with upcast).
+impl<T, Base> AsArg<Option<Gd<Base>>> for Option<&Gd<T>>
 where
-    T: GodotClass + Bounds<Declarer = bounds::DeclEngine>,
-    U: Inherits<T>,
+    T: Inherits<Base>,
+    Base: GodotClass + Bounds<Declarer = bounds::DeclEngine>,
 {
-    fn into_arg<'r>(self) -> CowArg<'r, Option<Gd<T>>> {
+    fn into_arg<'arg>(self) -> CowArg<'arg, Option<Gd<Base>>>
+    where
+        Self: 'arg,
+    {
+        // Upcasting to an owned value Gd<Base> requires cloning. Optimized path in into_ffi_arg().
         match self {
-            Some(gd) => CowArg::Owned(Some(gd.clone().upcast::<T>())),
+            Some(gd_ref) => AsArg::into_arg(gd_ref),
             None => CowArg::Owned(None),
         }
     }
-}
 
-impl<T, U> AsArg<Option<Gd<T>>> for Option<&Gd<U>>
-where
-    T: GodotClass + Bounds<Declarer = bounds::DeclEngine>,
-    U: Inherits<T>,
-{
-    fn into_arg<'cow>(self) -> CowArg<'cow, Option<Gd<T>>> {
-        // This needs to construct a new Option<Gd<T>>, so cloning is unavoidable
-        // since we go from Option<&Gd<U>> to Option<Gd<T>>
-        match self {
-            Some(gd) => CowArg::Owned(Some(gd.clone().upcast::<T>())),
-            None => CowArg::Owned(None),
-        }
+    fn into_ffi_arg<'arg>(self) -> CowArg<'arg, Option<Gd<Base>>>
+    where
+        Self: 'arg,
+    {
+        // SAFETY: ObjectArg exists only during FFI call.
+        let arg = unsafe { ObjectArg::from_option_gd(self) };
+        CowArg::FfiObject(arg)
     }
 }
 
-impl<T, U> AsArg<Option<Gd<T>>> for &Gd<U>
+/// Convert `&Gd` -> `Option<Gd>` (with upcast).
+impl<T, Base> AsArg<Option<Gd<Base>>> for &Gd<T>
 where
-    T: GodotClass + Bounds<Declarer = bounds::DeclEngine>,
-    U: Inherits<T>,
+    T: Inherits<Base>,
+    Base: GodotClass + Bounds<Declarer = bounds::DeclEngine>,
 {
-    fn into_arg<'cow>(self) -> CowArg<'cow, Option<Gd<T>>> {
-        CowArg::Owned(Some(self.clone().upcast::<T>()))
+    fn into_arg<'arg>(self) -> CowArg<'arg, Option<Gd<Base>>>
+    where
+        Self: 'arg,
+    {
+        // Upcasting to an owned value Gd<Base> requires cloning. Optimized path in into_ffi_arg().
+        CowArg::Owned(Some(self.clone().upcast::<Base>()))
+    }
+
+    fn into_ffi_arg<'arg>(self) -> CowArg<'arg, Option<Gd<Base>>>
+    where
+        Self: 'arg,
+    {
+        // SAFETY: ObjectArg exists only during FFI call.
+        let arg = unsafe { ObjectArg::from_gd(self) };
+        CowArg::FfiObject(arg)
     }
 }
 
-impl<T, U, D> AsArg<Option<Gd<T>>> for &DynGd<U, D>
+/// Convert `&DynGd` -> `Option<Gd>` (with upcast).
+impl<T, D, Base> AsArg<Option<Gd<Base>>> for &DynGd<T, D>
 where
-    T: GodotClass + Bounds<Declarer = bounds::DeclEngine>,
-    U: Inherits<T>,
+    T: Inherits<Base>,
     D: ?Sized,
+    Base: GodotClass + Bounds<Declarer = bounds::DeclEngine>,
 {
-    fn into_arg<'cow>(self) -> CowArg<'cow, Option<Gd<T>>> {
-        let gd: &Gd<U> = self; // Deref
-        CowArg::Owned(Some(gd.clone().upcast::<T>()))
+    fn into_arg<'arg>(self) -> CowArg<'arg, Option<Gd<Base>>>
+    where
+        Self: 'arg,
+    {
+        let gd_ref: &Gd<T> = self; // DynGd -> Gd deref.
+        AsArg::into_arg(gd_ref)
+    }
+
+    fn into_ffi_arg<'arg>(self) -> CowArg<'arg, Option<Gd<Base>>>
+    where
+        Self: 'arg,
+    {
+        let gd_ref: &Gd<T> = self; // DynGd -> Gd deref.
+        AsArg::into_ffi_arg(gd_ref)
     }
 }
 
@@ -251,9 +333,9 @@ where
 ///     }
 /// }
 /// ```
-pub fn owned_into_arg<'r, T>(owned_val: T) -> impl AsArg<T> + 'r
+pub fn owned_into_arg<'arg, T>(owned_val: T) -> impl AsArg<T> + 'arg
 where
-    T: ToGodot + 'r,
+    T: ToGodot + 'arg,
 {
     CowArg::Owned(owned_val)
 }
@@ -354,9 +436,9 @@ impl<T> AsArg<T> for CowArg<'_, T>
 where
     for<'r> T: ToGodot,
 {
-    fn into_arg<'r>(self) -> CowArg<'r, T>
+    fn into_arg<'arg>(self) -> CowArg<'arg, T>
     where
-        Self: 'r,
+        Self: 'arg,
     {
         self
     }
@@ -368,13 +450,13 @@ where
 // Note: for all string types S, `impl AsArg<S> for &mut String` is not yet provided, but we can add them if needed.
 
 impl AsArg<GString> for &str {
-    fn into_arg<'r>(self) -> CowArg<'r, GString> {
+    fn into_arg<'arg>(self) -> CowArg<'arg, GString> {
         CowArg::Owned(GString::from(self))
     }
 }
 
 impl AsArg<GString> for &String {
-    fn into_arg<'r>(self) -> CowArg<'r, GString> {
+    fn into_arg<'arg>(self) -> CowArg<'arg, GString> {
         CowArg::Owned(GString::from(self))
     }
 }
@@ -383,19 +465,19 @@ impl AsArg<GString> for &String {
 // StringName
 
 impl AsArg<StringName> for &str {
-    fn into_arg<'r>(self) -> CowArg<'r, StringName> {
+    fn into_arg<'arg>(self) -> CowArg<'arg, StringName> {
         CowArg::Owned(StringName::from(self))
     }
 }
 
 impl AsArg<StringName> for &String {
-    fn into_arg<'r>(self) -> CowArg<'r, StringName> {
+    fn into_arg<'arg>(self) -> CowArg<'arg, StringName> {
         CowArg::Owned(StringName::from(self))
     }
 }
 
 impl AsArg<StringName> for &'static CStr {
-    fn into_arg<'r>(self) -> CowArg<'r, StringName> {
+    fn into_arg<'arg>(self) -> CowArg<'arg, StringName> {
         CowArg::Owned(StringName::from(self))
     }
 }
@@ -404,13 +486,13 @@ impl AsArg<StringName> for &'static CStr {
 // NodePath
 
 impl AsArg<NodePath> for &str {
-    fn into_arg<'r>(self) -> CowArg<'r, NodePath> {
+    fn into_arg<'arg>(self) -> CowArg<'arg, NodePath> {
         CowArg::Owned(NodePath::from(self))
     }
 }
 
 impl AsArg<NodePath> for &String {
-    fn into_arg<'r>(self) -> CowArg<'r, NodePath> {
+    fn into_arg<'arg>(self) -> CowArg<'arg, NodePath> {
         CowArg::Owned(NodePath::from(self))
     }
 }