pyfisch / keyboard-types

Types to define keyboard related events.
Apache License 2.0
54 stars 9 forks source link

Improve code size of Key serde impls #6

Closed dtolnay closed 4 years ago

dtolnay commented 4 years ago

This PR drops the number of lines of LLVM IR involved in deserializing by 98.5%.

Before:

Lines   Function name
-----   -------------
47048   <keyboard_types::key::_IMPL_DESERIALIZE_FOR_Key::<impl serde::de::Deserialize for keyboard_types::key::Key>::deserialize::__Visitor as serde::de::Visitor>::visit_enum

After:

Lines   Function name
-----   -------------
  720   <<keyboard_types::key::Key as serde::de::Deserialize>::deserialize::KeyVisitor as serde::de::Visitor>::visit_enum

The total number of lines of LLVM IR in the following test program drops from 56244 to 8299:

fn main() {
    let arg = std::env::args().skip(1).next().unwrap();
    bincode::deserialize::<keyboard_types::Key>(arg.as_bytes()).unwrap();
}

You may be interested in making a similar change for some of the other big enums.

@SimonSapin https://github.com/serde-rs/serde/issues/1824

SimonSapin commented 4 years ago

While this is an impressive result, I’ve closed https://github.com/serde-rs/serde/issues/1824 now that updating bincode has reduced deserialization-related code size by 4×: https://github.com/servo/servo/pull/27063

With this reduced impact, I don’t know if it is worth maintaining a non-trivial manual Deserialize impl instead of deriving it. @pyfisch, what do you think?

pyfisch commented 4 years ago

With this reduced impact, I don’t know if it is worth maintaining a non-trivial manual Deserialize impl instead of deriving it. @pyfisch, what do you think?

If you don't think this is needed for servo, I will close this.