Skip to content

Commit

Permalink
Update bincode, reduce code size of script by ~11%
Browse files Browse the repository at this point in the history
… as measured in lines of unoptimized LLVM IR by

```
cargo llvm-lines --manifest-path ports/winit/Cargo.toml \
    -p script --features layout-2013 | head -n 30
```

… with https://github.com/dtolnay/cargo-llvm-lines
CC #26713

One of the top functions (in lines contribution) was a
`Visitor::visit_enum` method generated by `serde_derive` for deserializing
`keyboard_types::key::Key`, which is an enum with many variants.
I had filed serde-rs/serde#1824 to discuss this,
and dtolnay proposed pyfisch/keyboard-types#6
manually implementing `Deserialize` for that enum instead of deriving it.

However another point of note is that this function had four copies with
monomorphization. I could reproduce this in a program that does nothing
but deserialize an enum with bincode:
serde-rs/serde#1824 (comment)
This suggests that bincode uses instanciates the `Deserialize` impls
with four different `Deserializer` types which is three more than necessary.

https://dos.cafe/blog/bincode-1.3.html mentions changes to `Deserializer`
types in a new version of bincode. And sure enough, this issue is now fixed.

Before:

```
  Lines           Copies         Function name
  -----           ------         -------------
  7022161 (100%)  180367 (100%)  (TOTAL)
   178816 (2.5%)   15437 (8.6%)  core::ptr::drop_in_place
   151022 (2.2%)    6634 (3.7%)  core::ops::function::FnOnce::call_once
   122012 (1.7%)    1000 (0.6%)  <<&mut bincode::de::Deserializer<R,O> as serde::de::Deserializer>::deserialize_tuple::Access<R,O> as serde::de::SeqAccess>::next_element_seed
   101731 (1.4%)    1627 (0.9%)  core::option::Option<T>::map
    83196 (1.2%)    1192 (0.7%)  core::result::Result<T,E>::map
    51215 (0.7%)    6111 (3.4%)  core::ops::function::FnOnce::call_once{{vtable.shim}}
    47048 (0.7%)       4 (0.0%)  <keyboard_types::key::_IMPL_DESERIALIZE_FOR_Key::<impl serde::de::Deserialize for keyboard_types::key::Key>::deserialize::__Visitor as serde::de::Visitor>::visit_enum
    45388 (0.6%)     296 (0.2%)  <&mut bincode::de::Deserializer<R,O> as serde::de::Deserializer>::deserialize_enum::<impl serde::de::EnumAccess for &mut bincode::de::Deserializer<R,O>>::variant_seed
    44973 (0.6%)     647 (0.4%)  core::result::Result<T,E>::map_err
    39185 (0.6%)     516 (0.3%)  std::thread::local::LocalKey<T>::try_with
    39130 (0.6%)     215 (0.1%)  alloc::raw_vec::RawVec<T,A>::grow_amortized
    35334 (0.5%)     867 (0.5%)  alloc::alloc::box_free
    34788 (0.5%)     609 (0.3%)  crossbeam_channel::context::Context::with::{{closure}}
    33810 (0.5%)     646 (0.4%)  core::ptr::swap_nonoverlapping_one
    33610 (0.5%)     552 (0.3%)  core::result::Result<T,E>::unwrap_or_else
    32144 (0.5%)     392 (0.2%)  <*const T as core::fmt::Pointer>::fmt
    32062 (0.5%)     782 (0.4%)  script::dom::bindings::root::Root<T>::new
    31589 (0.4%)     600 (0.3%)  core::result::Result<T,E>::expect
    29364 (0.4%)     116 (0.1%)  <<script_traits::webdriver_msg::_IMPL_DESERIALIZE_FOR_WebDriverScriptCommand::<impl serde::de::Deserialize for script_traits::webdriver_msg::WebDriverScriptCommand>::deserialize::__Visitor as serde::de::Visitor>::visit_enum::__Visitor as serde::de::Visitor>::visit_seq
    26996 (0.4%)     346 (0.2%)  core::option::Option<T>::map_or
    26168 (0.4%)      92 (0.1%)  <<script_traits::_IMPL_DESERIALIZE_FOR_ConstellationControlMsg::<impl serde::de::Deserialize for script_traits::ConstellationControlMsg>::deserialize::__Visitor as serde::de::Visitor>::visit_enum::__Visitor as serde::de::Visitor>::visit_seq
    25996 (0.4%)     388 (0.2%)  script::dom::bindings::root::Root<script::dom::bindings::root::MaybeUnreflectedDom<T>>::reflect_with
    25833 (0.4%)     641 (0.4%)  core::result::Result<T,E>::unwrap
    25180 (0.4%)       4 (0.0%)  <keyboard_types::code::_IMPL_DESERIALIZE_FOR_Code::<impl serde::de::Deserialize for keyboard_types::code::Code>::deserialize::__Visitor as serde::de::Visitor>::visit_enum
    24751 (0.4%)     197 (0.1%)  core::iter::traits::iterator::Iterator::try_fold
    22216 (0.3%)     376 (0.2%)  bincode::internal::deserialize_seed
    22185 (0.3%)     145 (0.1%)  hashbrown::raw::RawTable<T>::find
```

After:

```
  Lines           Copies         Function name
  -----           ------         -------------
  6239581 (100%)  169803 (100%)  (TOTAL)
   178770 (2.9%)   15434 (9.1%)  core::ptr::drop_in_place
   151022 (2.4%)    6634 (3.9%)  core::ops::function::FnOnce::call_once
   101146 (1.6%)    1618 (1.0%)  core::option::Option<T>::map
    76938 (1.2%)    1090 (0.6%)  core::result::Result<T,E>::map
    51215 (0.8%)    6111 (3.6%)  core::ops::function::FnOnce::call_once{{vtable.shim}}
    44973 (0.7%)     647 (0.4%)  core::result::Result<T,E>::map_err
    39185 (0.6%)     516 (0.3%)  std::thread::local::LocalKey<T>::try_with
    39130 (0.6%)     215 (0.1%)  alloc::raw_vec::RawVec<T,A>::grow_amortized
    35334 (0.6%)     867 (0.5%)  alloc::alloc::box_free
    34788 (0.6%)     609 (0.4%)  crossbeam_channel::context::Context::with::{{closure}}
    33810 (0.5%)     646 (0.4%)  core::ptr::swap_nonoverlapping_one
    33610 (0.5%)     552 (0.3%)  core::result::Result<T,E>::unwrap_or_else
    32144 (0.5%)     392 (0.2%)  <*const T as core::fmt::Pointer>::fmt
    32062 (0.5%)     782 (0.5%)  script::dom::bindings::root::Root<T>::new
    31589 (0.5%)     600 (0.4%)  core::result::Result<T,E>::expect
    30069 (0.5%)     250 (0.1%)  <<&mut bincode::de::Deserializer<R,O> as serde::de::Deserializer>::deserialize_tuple::Access<R,O> as serde::de::SeqAccess>::next_element_seed
    26996 (0.4%)     346 (0.2%)  core::option::Option<T>::map_or
    25996 (0.4%)     388 (0.2%)  script::dom::bindings::root::Root<script::dom::bindings::root::MaybeUnreflectedDom<T>>::reflect_with
    25833 (0.4%)     641 (0.4%)  core::result::Result<T,E>::unwrap
    24751 (0.4%)     197 (0.1%)  core::iter::traits::iterator::Iterator::try_fold
    22185 (0.4%)     145 (0.1%)  hashbrown::raw::RawTable<T>::find
    22000 (0.4%)      80 (0.0%)  hashbrown::raw::RawTable<T>::rehash_in_place
    20808 (0.3%)     289 (0.2%)  core::alloc::layout::Layout::array
    20031 (0.3%)     308 (0.2%)  core::option::Option<T>::ok_or_else
    19732 (0.3%)       2 (0.0%)  html5ever::tree_builder::TreeBuilder<Handle,Sink>::step
    18951 (0.3%)    1014 (0.6%)  core::ptr::read
    17261 (0.3%)     201 (0.1%)  core::iter::traits::iterator::Iterator::fold
```

That `visit_enum` for `Key` is not in the first 30 lines, I find it after
changing the filtering command to `head -n 60`:

```
    11762 (0.2%)       1 (0.0%)  <keyboard_types::key::_IMPL_DESERIALIZE_FOR_Key::<impl serde::de::Deserialize for keyboard_types::key::Key>::deserialize::__Visitor as serde::de::Visitor>::visit_enum
```

It has one copy / instantiation instead of four, which contributes exactly
four times fewer lines of IR.

The crate total reduced by ~782k lines, a lot more than ~35k for this
`visit_enum` function. We see that `next_element_seed` (also for bincode
deserialization) now has 250 copies instead of 1000. So it looks like
*everything* bincode related was reduced by 4×
  • Loading branch information
SimonSapin committed Jun 24, 2020
1 parent 0838d78 commit a215ef3
Showing 1 changed file with 2 additions and 2 deletions.
4 changes: 2 additions & 2 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit a215ef3

Please sign in to comment.