dbus2 / zbus

Rust D-Bus crate.
Other
359 stars 82 forks source link

Make `zvariant::Signature` completely static & turn `Type::signature` to trait const #882

Closed zeenix closed 3 weeks ago

zeenix commented 3 months ago

When I wrote zvariant, I was still very new to Rust and didn't make the best choices in some regard. Recently I looked into the postcard-rpc code, and that project also seem to have something very similar to a signature: a Schema. However, unlike our Signature type, Schema is completely static and allocation free (it's targeted for microcontrollers). So it got me thinking, if our Signature was similar, we could not only avoid a lot of allocations, but greatly optimize the ser/de process (according to the last benchmarks I conducted, signature parsing is what costs us the most, especially for array types). So something like this:

#[derive(Debug, Clone, Copy)]
pub enum Signature {
    // Simple types
    U8,
    Bool,
    I16,
    U16,
    I32,
    U32,
    I64,
    U64,
    F64,
    Str,
    Signature,
    ObjectPath,
    Value,
    #[cfg(unix)]
    Fd,

    // Container types
    Array {
        child: &'static Signature,
    },
    Dict {
        key: &'static Signature,
        value: &'static Signature,
    },
    Structure {
        fields: &'static [Signature],
    },
    #[cfg(feature = "gvariant")]
    Maybe {
        child: &'static Signature,
    },
}

pub trait Type {
    const SIGNATURE: Signature;
}

impl<T: Type> Type for [T] {
    const SIGNATURE: Signature = Signature::Array {
        child: &T::SIGNATURE,
    };
}

impl<T: Type> Type for (T,) {
    const SIGNATURE: Signature = Signature::Structure {
        fields: &[T::SIGNATURE],
    };
}
// TODO: Use a macro for for generating all tuple impls

All the above code works. :cherries: on top if we can make this work too:

impl Signature {
    pub const fn as_str(&self) -> &'static str {
        match self {
            Signature::U8 => "y",
            Signature::Bool => "b",
            Signature::I16 => "n",
            Signature::U16 => "q",
            Signature::I32 => "i",
            Signature::U32 => "u",
            Signature::I64 => "x",
            Signature::U64 => "t",
            Signature::F64 => "d",
            Signature::Str => "s",
            Signature::Signature => "g",
            Signature::ObjectPath => "o",
            Signature::Value => "v",
            #[cfg(unix)]
            Signature::Fd => "h",
            Signature::Array { child } => {
                concat_const::concat!("a", child.as_str())
            }
            Signature::Dict { key, value } => {
                concat_const::concat!("a{", key.as_str(), value.as_str(), "}")
            }
            Signature::Structure { fields } => {
                let fields = fields
                    .iter()
                    .map(|f| f.as_str())
                    .collect::<[]>()
                    .join("");
                &format!("({})", fields)
            }
            #[cfg(feature = "gvariant")]
            Signature::Maybe { child } => {
                let child = child.as_str();
                concat_const::concat!("m{}", child)
            }
        }
    }
}

but I suspect it's not possible and one part that will require allocation. However, with Display implementation, we can support writing to fixed sized buffers too.

Another possible hurdle could be zvariant::DynamicType. Maybe there could also be a DynamicSignature and/or Signature could have lifetime. Something I need to figure out..


Of course, this all would mean breaking API.

zeenix commented 3 months ago

After a lot of different approaches, I finally came to a solution that works and I'm happy with:

https://github.com/zeenix/experiments/blob/a6b790a4619eacd5885111c8f4bbe371f36cec62/static-dbus-signature/src/signature.rs https://github.com/zeenix/experiments/blob/a6b790a4619eacd5885111c8f4bbe371f36cec62/static-dbus-signature/src/type.rs https://github.com/zeenix/experiments/blob/a6b790a4619eacd5885111c8f4bbe371f36cec62/static-dbus-signature/src/dynamic_type.rs

It's a nice compromise where Type impls (which is most of the cases) always have a completely const signature but anything needing dynamic signature needs to allocate. Perhaps it would be best to use Arc<[Signature]> but that's a detail.

I'll need to check signature use/needs in the code base to see if this will work for zbus needs but I'm feeling confident.

zeenix commented 1 month ago

Quoting from #966

This PR addresses most of #882. Completely addressing it would require breaking API so it has to wait for the next major release.

Actually, I realized that this isn't true. The only reason Type::parsed_signature can't be a const, is that the Type macro needs to parse its string form (if user provides the signature through the attribute) and currently that can't be done because of circular dep it will create between zvariant and zvariant-derive.

This makes it important to do this already before the next 3.0 release. Here is the rough TODO:

zeenix commented 1 month ago

This PR addresses most of #882. Completely addressing it would require breaking API so it has to wait for the next major release.

Actually, I realized that this isn't true. The only reason Type::parsed_signature can't be a const, is that the Type macro needs to parse its string form (if user provides the signature through the attribute) and currently that can't be done because of circular dep it will create between zvariant and zvariant-derive.

oh actually, no. :laughing: The other reason is that adding a const w/o a default value to public trait (Type) would a breaking change and the default value will have to parse the return value of signature() and we can't do that in a const context. Hence this change, while not too difficult (probably will only take a day), is still going to be a breaking change.