Mirrors/servo
1
0
Fork 0
mirror of https://github.com/servo/servo.git synced 2025-08-06 06:00:15 +01:00
Code Issues Projects Releases Packages Wiki Activity

Auto merge of #27063 - servo:bincodeup, r=nox

Browse source 
Update bincode, reduce code size of `script` by ~11%

… 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 https://github.com/servo/servo/issues/26713

One of the top functions (in lines contribution) was a `Visitor::visit_enum` method generated by `serde_derive` for deserializing `keyboard_types:🔑:Key`, which is an enum with many variants. I had filed https://github.com/serde-rs/serde/issues/1824 to discuss this, and dtolnay proposed https://github.com/pyfisch/keyboard-types/pull/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: https://github.com/serde-rs/serde/issues/1824#issuecomment-641244476 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:🇩🇪:Deserializer<R,O> as serde:🇩🇪:Deserializer>::deserialize_tuple::Access<R,O> as serde:🇩🇪: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:🔑:_IMPL_DESERIALIZE_FOR_Key::<impl serde:🇩🇪:Deserialize for keyboard_types:🔑:Key>::deserialize::__Visitor as serde:🇩🇪:Visitor>::visit_enum
    45388 (0.6%)     296 (0.2%)  <&mut bincode:🇩🇪:Deserializer<R,O> as serde:🇩🇪:Deserializer>::deserialize_enum::<impl serde:🇩🇪:EnumAccess for &mut bincode:🇩🇪: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:🧵: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:🇩🇪:Deserialize for script_traits::webdriver_msg::WebDriverScriptCommand>::deserialize::__Visitor as serde:🇩🇪:Visitor>::visit_enum::__Visitor as serde:🇩🇪: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:🇩🇪:Deserialize for script_traits::ConstellationControlMsg>::deserialize::__Visitor as serde:🇩🇪:Visitor>::visit_enum::__Visitor as serde:🇩🇪: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:🇩🇪:Deserialize for keyboard_types::code::Code>::deserialize::__Visitor as serde:🇩🇪: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:🧵: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:🇩🇪:Deserializer<R,O> as serde:🇩🇪:Deserializer>::deserialize_tuple::Access<R,O> as serde:🇩🇪: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:🔑:_IMPL_DESERIALIZE_FOR_Key::<impl serde:🇩🇪:Deserialize for keyboard_types:🔑:Key>::deserialize::__Visitor as serde:🇩🇪:Visitor>::visit_enum
```

It has one copy / instantiation instead of four, and 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 also 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×
This commit is contained in:
bors-servo 2020-06-24 08:09:51 -04:00 • committed by GitHub
commit 6fc77add11
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
1 changed files with 2 additions and 2 deletions
Download patch file Download diff file Expand all files Collapse all files

4
Cargo.lock generated
View file

@ -260,9 +260,9 @@ checksum = "88ceb0d16c4fd0e42876e298d7d3ce3780dd9ebdcbe4199816a32c77e08597ff"
[[package]]
name = "bincode"
version = "1.2.1"
version = "1.3.1"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "5753e2a71534719bf3f4e57006c3a4f0d2c672a4b676eec84161f763eca87dbf"
checksum = "f30d3a39baa26f9651f17b375061f3233dde33424a8b72b0dbe93a68a0bc896d"
dependencies = [
"byteorder",
"serde",