From 8ff681924203e682bdf602bb601a3f2dec8380ed Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= <emilio@crisal.io>
Date: Mon, 6 Jan 2025 14:15:03 +0100
Subject: [PATCH] =?UTF-8?q?=E2=9A=A1=EF=B8=8F=20zb,zm:=20Make=20header=20c?=
 =?UTF-8?q?loning=20cheaper=20when=20it=20comes=20from=20a=20message=20by?=
 =?UTF-8?q?=20using=20Cow<Signature>.?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Right now cloning a header involves cloning the `PrimaryHeader` (cheap,
mostly POD), and then cloning the `Fields`, which can be somewhat
complex, as it involves cloning all the strings and the signature, which
if dynamic could be rather costly.

From things coming from a message, strings are always `Borrowed` so the
cloning is cheap.

So the only expensive thing is cloning the signature. Using `Cow` for it
looks sensible, it's what `Str` is doing internally already effectively
(with some extra tweaks).
---
 zbus/src/message/builder.rs | 5 +++--
 zbus/src/message/fields.rs  | 9 +++++----
 zbus/src/message/header.rs  | 4 ++--
 zbus/src/message/mod.rs     | 4 ++--
 4 files changed, 12 insertions(+), 10 deletions(-)

diff --git a/zbus/src/message/builder.rs b/zbus/src/message/builder.rs
index 2c6bb97be..6f40908e3 100644
--- a/zbus/src/message/builder.rs
+++ b/zbus/src/message/builder.rs
@@ -1,4 +1,5 @@
 use std::{
+    borrow::Cow,
     io::{Cursor, Write},
     sync::Arc,
 };
@@ -231,7 +232,7 @@ impl<'a> Builder<'a> {
         let ctxt = dbus_context!(self, 0);
         let mut header = self.header;
 
-        header.fields_mut().signature = signature;
+        header.fields_mut().signature = Cow::Owned(signature);
 
         let body_len_u32 = body_size.size().try_into().map_err(|_| Error::ExcessData)?;
         header.primary_mut().set_body_len(body_len_u32);
@@ -285,7 +286,7 @@ impl<'m> From<Header<'m>> for Builder<'m> {
     fn from(mut header: Header<'m>) -> Self {
         // Signature and Fds are added by body* methods.
         let fields = header.fields_mut();
-        fields.signature = Signature::Unit;
+        fields.signature = Cow::Owned(Signature::Unit);
         fields.unix_fds = None;
 
         Self { header }
diff --git a/zbus/src/message/fields.rs b/zbus/src/message/fields.rs
index 8cda45414..d2591100c 100644
--- a/zbus/src/message/fields.rs
+++ b/zbus/src/message/fields.rs
@@ -4,7 +4,7 @@ use serde::{
     Deserialize, Deserializer, Serialize, Serializer,
 };
 use static_assertions::assert_impl_all;
-use std::num::NonZeroU32;
+use std::{borrow::Cow, num::NonZeroU32};
 use zbus_names::{BusName, ErrorName, InterfaceName, MemberName, UniqueName};
 use zvariant::{ObjectPath, Signature, Type, Value};
 
@@ -23,7 +23,7 @@ pub(crate) struct Fields<'f> {
     pub reply_serial: Option<NonZeroU32>,
     pub destination: Option<BusName<'f>>,
     pub sender: Option<UniqueName<'f>>,
-    pub signature: Signature,
+    pub signature: Cow<'f, Signature>,
     pub unix_fds: Option<u32>,
 }
 
@@ -63,7 +63,7 @@ impl Serialize for Fields<'_> {
         if let Some(sender) = &self.sender {
             seq.serialize_element(&(FieldCode::Sender, Value::from(sender.as_str())))?;
         }
-        if !matches!(&self.signature, Signature::Unit) {
+        if !matches!(&*self.signature, Signature::Unit) {
             seq.serialize_element(&(FieldCode::Signature, SignatureSerializer(&self.signature)))?;
         }
         if let Some(unix_fds) = self.unix_fds {
@@ -149,7 +149,8 @@ impl<'de> Visitor<'de> for FieldsVisitor {
                     fields.sender = Some(UniqueName::try_from(value).map_err(V::Error::custom)?)
                 }
                 FieldCode::Signature => {
-                    fields.signature = Signature::try_from(value).map_err(V::Error::custom)?
+                    fields.signature =
+                        Cow::Owned(Signature::try_from(value).map_err(V::Error::custom)?)
                 }
                 FieldCode::UnixFDs => {
                     fields.unix_fds = Some(u32::try_from(value).map_err(V::Error::custom)?)
diff --git a/zbus/src/message/header.rs b/zbus/src/message/header.rs
index 7257526b7..4039fb824 100644
--- a/zbus/src/message/header.rs
+++ b/zbus/src/message/header.rs
@@ -329,7 +329,7 @@ static SERIAL_NUM: AtomicU32 = AtomicU32::new(0);
 mod tests {
     use crate::message::{Fields, Header, PrimaryHeader, Type};
 
-    use std::error::Error;
+    use std::{borrow::Cow, error::Error};
     use test_log::test;
     use zbus_names::{InterfaceName, MemberName};
     use zvariant::{ObjectPath, Signature};
@@ -361,7 +361,7 @@ mod tests {
         f.error_name = Some("org.zbus.Error".try_into()?);
         f.destination = Some(":1.11".try_into()?);
         f.reply_serial = Some(88.try_into()?);
-        f.signature = "say".try_into().unwrap();
+        f.signature = Cow::Owned("say".try_into().unwrap());
         f.unix_fds = Some(12);
         let h = Header::new(PrimaryHeader::new(Type::MethodReturn, 77), f);
 
diff --git a/zbus/src/message/mod.rs b/zbus/src/message/mod.rs
index 14df3a5c2..a238d3d2f 100644
--- a/zbus/src/message/mod.rs
+++ b/zbus/src/message/mod.rs
@@ -1,5 +1,5 @@
 //! D-Bus Message.
-use std::{fmt, sync::Arc};
+use std::{borrow::Cow, fmt, sync::Arc};
 
 use static_assertions::assert_impl_all;
 use zbus_names::{ErrorName, InterfaceName, MemberName};
@@ -176,7 +176,7 @@ impl Message {
             reply_serial: quick_fields.reply_serial(),
             destination: quick_fields.destination(self),
             sender: quick_fields.sender(self),
-            signature: quick_fields.signature().clone(),
+            signature: Cow::Borrowed(quick_fields.signature()),
             unix_fds: quick_fields.unix_fds(),
         };