Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Browse files
Browse the repository at this point in the history
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 #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