Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Shorten serde-derive generated code #2310

Closed
kangalio opened this issue Nov 4, 2022 · 2 comments
Closed

Shorten serde-derive generated code #2310

kangalio opened this issue Nov 4, 2022 · 2 comments

Comments

@kangalio
Copy link

kangalio commented Nov 4, 2022

In the serenity community, we made some quick tests and noticed that derived Serialize/Deserialize implementations make up ~75% of base compile time. I think there is improvement potential.

A simple struct:

#[derive(serde::Deserialize)]
struct Foo {
    foo: String,
}
Generates a mountain of code:
#[doc(hidden)]
#[allow(non_upper_case_globals, unused_attributes, unused_qualifications)]
const _: () =
    {
        #[allow(unused_extern_crates, clippy :: useless_attribute)]
        extern crate serde as _serde;
        #[allow(unused_macros)]
        macro_rules! try {
            ($__expr : expr) =>
            {
                match $__expr
                {
                    _serde :: __private :: Ok(__val) => __val, _serde ::
                    __private :: Err(__err) =>
                    { return _serde :: __private :: Err(__err) ; }
                }
            }
        }
        #[automatically_derived]
        impl<'de> _serde::Deserialize<'de> for Foo {
            fn deserialize<__D>(__deserializer: __D)
                -> _serde::__private::Result<Self, __D::Error> where
                __D: _serde::Deserializer<'de> {
                #[allow(non_camel_case_types)]
                enum __Field { __field0, __ignore, }
                struct __FieldVisitor;
                impl<'de> _serde::de::Visitor<'de> for __FieldVisitor {
                    type Value = __Field;
                    fn expecting(&self,
                        __formatter: &mut _serde::__private::Formatter)
                        -> _serde::__private::fmt::Result {
                        _serde::__private::Formatter::write_str(__formatter,
                            "field identifier")
                    }
                    fn visit_u64<__E>(self, __value: u64)
                        -> _serde::__private::Result<Self::Value, __E> where
                        __E: _serde::de::Error {
                        match __value {
                            0u64 => _serde::__private::Ok(__Field::__field0),
                            _ => _serde::__private::Ok(__Field::__ignore),
                        }
                    }
                    fn visit_str<__E>(self, __value: &str)
                        -> _serde::__private::Result<Self::Value, __E> where
                        __E: _serde::de::Error {
                        match __value {
                            "foo" => _serde::__private::Ok(__Field::__field0),
                            _ => { _serde::__private::Ok(__Field::__ignore) }
                        }
                    }
                    fn visit_bytes<__E>(self, __value: &[u8])
                        -> _serde::__private::Result<Self::Value, __E> where
                        __E: _serde::de::Error {
                        match __value {
                            b"foo" => _serde::__private::Ok(__Field::__field0),
                            _ => { _serde::__private::Ok(__Field::__ignore) }
                        }
                    }
                }
                impl<'de> _serde::Deserialize<'de> for __Field {
                    #[inline]
                    fn deserialize<__D>(__deserializer: __D)
                        -> _serde::__private::Result<Self, __D::Error> where
                        __D: _serde::Deserializer<'de> {
                        _serde::Deserializer::deserialize_identifier(__deserializer,
                            __FieldVisitor)
                    }
                }
                struct __Visitor<'de> {
                    marker: _serde::__private::PhantomData<Foo>,
                    lifetime: _serde::__private::PhantomData<&'de ()>,
                }
                impl<'de> _serde::de::Visitor<'de> for __Visitor<'de> {
                    type Value = Foo;
                    fn expecting(&self,
                        __formatter: &mut _serde::__private::Formatter)
                        -> _serde::__private::fmt::Result {
                        _serde::__private::Formatter::write_str(__formatter,
                            "struct Foo")
                    }
                    #[inline]
                    fn visit_seq<__A>(self, mut __seq: __A)
                        -> _serde::__private::Result<Self::Value, __A::Error> where
                        __A: _serde::de::SeqAccess<'de> {
                        let __field0 =
                            match match _serde::de::SeqAccess::next_element::<String>(&mut __seq)
                                    {
                                    _serde::__private::Ok(__val) => __val,
                                    _serde::__private::Err(__err) => {
                                        return _serde::__private::Err(__err);
                                    }
                                } {
                                _serde::__private::Some(__value) => __value,
                                _serde::__private::None => {
                                    return _serde::__private::Err(_serde::de::Error::invalid_length(0usize,
                                                &"struct Foo with 1 element"));
                                }
                            };
                        _serde::__private::Ok(Foo { foo: __field0 })
                    }
                    #[inline]
                    fn visit_map<__A>(self, mut __map: __A)
                        -> _serde::__private::Result<Self::Value, __A::Error> where
                        __A: _serde::de::MapAccess<'de> {
                        let mut __field0: _serde::__private::Option<String> =
                            _serde::__private::None;
                        while let _serde::__private::Some(__key) =
                                match _serde::de::MapAccess::next_key::<__Field>(&mut __map)
                                    {
                                    _serde::__private::Ok(__val) => __val,
                                    _serde::__private::Err(__err) => {
                                        return _serde::__private::Err(__err);
                                    }
                                } {
                            match __key {
                                __Field::__field0 => {
                                    if _serde::__private::Option::is_some(&__field0) {
                                            return _serde::__private::Err(<__A::Error as
                                                            _serde::de::Error>::duplicate_field("foo"));
                                        }
                                    __field0 =
                                        _serde::__private::Some(match _serde::de::MapAccess::next_value::<String>(&mut __map)
                                                {
                                                _serde::__private::Ok(__val) => __val,
                                                _serde::__private::Err(__err) => {
                                                    return _serde::__private::Err(__err);
                                                }
                                            });
                                }
                                _ => {
                                    let _ =
                                        match _serde::de::MapAccess::next_value::<_serde::de::IgnoredAny>(&mut __map)
                                            {
                                            _serde::__private::Ok(__val) => __val,
                                            _serde::__private::Err(__err) => {
                                                return _serde::__private::Err(__err);
                                            }
                                        };
                                }
                            }
                        }
                        let __field0 =
                            match __field0 {
                                _serde::__private::Some(__field0) => __field0,
                                _serde::__private::None =>
                                    match _serde::__private::de::missing_field("foo") {
                                        _serde::__private::Ok(__val) => __val,
                                        _serde::__private::Err(__err) => {
                                            return _serde::__private::Err(__err);
                                        }
                                    },
                            };
                        _serde::__private::Ok(Foo { foo: __field0 })
                    }
                }
                const FIELDS: &'static [&'static str] = &["foo"];
                _serde::Deserializer::deserialize_struct(__deserializer,
                    "Foo", FIELDS,
                    __Visitor {
                        marker: _serde::__private::PhantomData::<Foo>,
                        lifetime: _serde::__private::PhantomData,
                    })
            }
        }
    };

Playground link (run Tools -> Expand Macros)

It feels like this can be reduced a lot, perhaps by moving repetitive code into #[doc(hidden)] public functions in serde, which the derive-generated code calls into.

Related: #1146 (comment), #286

@krischal111
Copy link

Yes, I support this. I am writing a project, and it takes so long to compile.

I don't know much about how this particular macro works. But if the derive macros for Deserialize implements all the type it supports, it would be very much time consuming.

It could be implemented functionalities like:

#[derive(serde::deserialize::Json)]
struct Foo {
    foo: String,
}

And if you want to implement for all it should be like:
#[derive(serde::deserialize::All)]

So, perhaps it would be better to add option to implement Deserialize for json or our specific use cases only.

@dtolnay
Copy link
Member

dtolnay commented Jul 9, 2023

I don't think this issue contains actionable information, because in general the length of the generated code is not usually the issue. Above, you showed 165 lines of serde_derive-generated code to illustrate how it's verbose, but there are good reasons for macro-generated code to be verbose relative to handwritten code (hygiene) and some of those reasons improve compile time, in contrast to what one might expect by focusing on length of code (less reliance on name resolution, less reliance on type conversions, fewer type inference variables, less use of generic closure-based APIs, fewer function calls, fewer autoref or autoderef operations, ...).

To illustrate this, here is a 100% equivalent impl in handwritten style in 57 lines (⅓ as many) that takes longer to compile.

impl<'de> serde::Deserialize<'de> for Foo {
    fn deserialize<D: serde::Deserializer<'de>>(deserializer: D) -> Result<Self, D::Error> {
        struct Field(u64);
        struct FieldVisitor;
        impl<'de> serde::de::Visitor<'de> for FieldVisitor {
            type Value = Field;
            fn expecting(&self, formatter: &mut std::fmt::Formatter) -> std::fmt::Result {
                formatter.write_str("field identifier")
            }
            fn visit_u64<E: serde::de::Error>(self, v: u64) -> Result<Field, E> {
                Ok(Field(v))
            }
            fn visit_str<E: serde::de::Error>(self, v: &str) -> Result<Field, E> {
                self.visit_bytes(v.as_bytes())
            }
            fn visit_bytes<E: serde::de::Error>(self, v: &[u8]) -> Result<Field, E> {
                Ok(Field(if v == b"foo" { 0 } else { 1 }))
            }
        }
        impl<'de> serde::Deserialize<'de> for Field {
            fn deserialize<D: serde::Deserializer<'de>>(deserializer: D) -> Result<Self, D::Error> {
                deserializer.deserialize_identifier(FieldVisitor)
            }
        }
        struct FooVisitor;
        impl<'de> serde::de::Visitor<'de> for FooVisitor {
            type Value = Foo;
            fn expecting(&self, formatter: &mut std::fmt::Formatter) -> std::fmt::Result {
                formatter.write_str("struct Foo")
            }
            fn visit_seq<A: serde::de::SeqAccess<'de>>(self, mut seq: A) -> Result<Foo, A::Error> {
                let foo = seq.next_element()?.ok_or_else(|| {
                    serde::de::Error::invalid_length(0, &"struct Foo with 1 element")
                })?;
                Ok(Foo { foo })
            }
            fn visit_map<A: serde::de::MapAccess<'de>>(self, mut map: A) -> Result<Foo, A::Error> {
                let mut foo = None;
                while let Some(field) = map.next_key()? {
                    if let Field(0) = field {
                        if foo.is_some() {
                            return Err(serde::de::Error::duplicate_field("foo"));
                        }
                        foo = map.next_value().map(Some)?;
                    } else {
                        map.next_value::<serde::de::IgnoredAny>()?;
                    }
                }
                if foo.is_none() {
                    foo = serde::__private::de::missing_field("foo").map(Some)?;
                }
                Ok(Foo { foo: foo.unwrap() })
            }
        }
        deserializer.deserialize_struct("Foo", &["foo"], FooVisitor)
    }
}

@dtolnay dtolnay closed this as completed Jul 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

3 participants