From a215ef351a8ee49a4188220021756f1d0eca4c71 Mon Sep 17 00:00:00 2001 From: Simon Sapin Date: Wed, 24 Jun 2020 09:30:53 +0200 Subject: [PATCH] Update bincode, reduce code size of `script` by ~11% MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit … 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::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::de::Deserializer as serde::de::Deserializer>::deserialize_tuple::Access as serde::de::SeqAccess>::next_element_seed 101731 (1.4%) 1627 (0.9%) core::option::Option::map 83196 (1.2%) 1192 (0.7%) core::result::Result::map 51215 (0.7%) 6111 (3.4%) core::ops::function::FnOnce::call_once{{vtable.shim}} 47048 (0.7%) 4 (0.0%) ::deserialize::__Visitor as serde::de::Visitor>::visit_enum 45388 (0.6%) 296 (0.2%) <&mut bincode::de::Deserializer as serde::de::Deserializer>::deserialize_enum::>::variant_seed 44973 (0.6%) 647 (0.4%) core::result::Result::map_err 39185 (0.6%) 516 (0.3%) std::thread::local::LocalKey::try_with 39130 (0.6%) 215 (0.1%) alloc::raw_vec::RawVec::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::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::new 31589 (0.4%) 600 (0.3%) core::result::Result::expect 29364 (0.4%) 116 (0.1%) <::deserialize::__Visitor as serde::de::Visitor>::visit_enum::__Visitor as serde::de::Visitor>::visit_seq 26996 (0.4%) 346 (0.2%) core::option::Option::map_or 26168 (0.4%) 92 (0.1%) <::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>::reflect_with 25833 (0.4%) 641 (0.4%) core::result::Result::unwrap 25180 (0.4%) 4 (0.0%) ::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::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::map 76938 (1.2%) 1090 (0.6%) core::result::Result::map 51215 (0.8%) 6111 (3.6%) core::ops::function::FnOnce::call_once{{vtable.shim}} 44973 (0.7%) 647 (0.4%) core::result::Result::map_err 39185 (0.6%) 516 (0.3%) std::thread::local::LocalKey::try_with 39130 (0.6%) 215 (0.1%) alloc::raw_vec::RawVec::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::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::new 31589 (0.5%) 600 (0.4%) core::result::Result::expect 30069 (0.5%) 250 (0.1%) <<&mut bincode::de::Deserializer as serde::de::Deserializer>::deserialize_tuple::Access as serde::de::SeqAccess>::next_element_seed 26996 (0.4%) 346 (0.2%) core::option::Option::map_or 25996 (0.4%) 388 (0.2%) script::dom::bindings::root::Root>::reflect_with 25833 (0.4%) 641 (0.4%) core::result::Result::unwrap 24751 (0.4%) 197 (0.1%) core::iter::traits::iterator::Iterator::try_fold 22185 (0.4%) 145 (0.1%) hashbrown::raw::RawTable::find 22000 (0.4%) 80 (0.0%) hashbrown::raw::RawTable::rehash_in_place 20808 (0.3%) 289 (0.2%) core::alloc::layout::Layout::array 20031 (0.3%) 308 (0.2%) core::option::Option::ok_or_else 19732 (0.3%) 2 (0.0%) html5ever::tree_builder::TreeBuilder::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%) ::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× --- Cargo.lock | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index ab35d21c54f..8849f2165f7 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -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",