Skip to content

Commit

Permalink
⚡️ zb: Make header cloning cheaper when it comes from a message by us…
Browse files Browse the repository at this point in the history
…ing Cow<Signature>.

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).
  • Loading branch information
emilio committed Jan 6, 2025
1 parent 2181232 commit 6902605
Show file tree
Hide file tree
Showing 4 changed files with 12 additions and 10 deletions.
5 changes: 3 additions & 2 deletions zbus/src/message/builder.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use std::{
borrow::Cow,
io::{Cursor, Write},
sync::Arc,
};
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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 }
Expand Down
9 changes: 5 additions & 4 deletions zbus/src/message/fields.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};

Expand All @@ -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>,
}

Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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)?)
Expand Down
4 changes: 2 additions & 2 deletions zbus/src/message/header.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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);

Expand Down
4 changes: 2 additions & 2 deletions zbus/src/message/mod.rs
Original file line number Diff line number Diff line change
@@ -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};
Expand Down Expand Up @@ -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(),
};

Expand Down

0 comments on commit 6902605

Please sign in to comment.