From 2208a0f5948607cd50d551334f9aa099a3730cd8 Mon Sep 17 00:00:00 2001 From: DaniPopes <57450786+DaniPopes@users.noreply.github.com> Date: Wed, 18 Sep 2024 00:15:18 +0200 Subject: [PATCH] Reduce monomorphization in Deserializer implementation The `F` generic in `MapDe` and `SeqDe` makes it so that every constructed instance has its own type, which creates unnecessary monomorphizations of the underlying `Deserialize` methods. This is especially noticeable when the type being deserialized is a struct with 100+ fields that automatically derives the `Deserialize` implementation, as the multiple copies of the `visit_map` function end up being up to hundreds of KiB, bloating code size and compilation time. Here's an example output of [`cargo-bloat`] on such a configuration: ```text ... 0.0% 0.1% 64.2KiB foundry_config <::deserialize::__Visitor as serde::de::Visitor>::visit_map:: as figment::value::magic::Magic>::deserialize_from<::deserialize::__Visitor, figment::value::de::DefaultInterpreter>::{closure#1}>> 0.0% 0.1% 64.2KiB foundry_config <::deserialize::__Visitor as serde::de::Visitor>::visit_map:: as figment::value::magic::Magic>::deserialize_from<::deserialize::__Visitor, figment::value::de::DefaultInterpreter>::{closure#2}>> 0.0% 0.1% 64.2KiB foundry_config <::deserialize::__Visitor as serde::de::Visitor>::visit_map::::deserialize_from<::deserialize::__Visitor, figment::value::de::DefaultInterpreter>::{closure#2}>> 0.0% 0.1% 64.2KiB foundry_config <::deserialize::__Visitor as serde::de::Visitor>::visit_map::::deserialize_from<::deserialize::__Visitor, figment::value::de::DefaultInterpreter>::{closure#0}>> 0.0% 0.1% 64.2KiB foundry_config <::deserialize::__Visitor as serde::de::Visitor>::visit_map::::deserialize_from<::deserialize::__Visitor, figment::value::de::DefaultInterpreter>::{closure#0}>> 0.0% 0.1% 64.0KiB foundry_config <::deserialize::__Visitor as serde::de::Visitor>::visit_map:: as figment::value::magic::Magic>::deserialize_from<::deserialize::__Visitor, figment::value::de::DefaultInterpreter>::{closure#1}>> 0.0% 0.1% 64.0KiB foundry_config <::deserialize::__Visitor as serde::de::Visitor>::visit_map:: as figment::value::magic::Magic>::deserialize_from<::deserialize::__Visitor, figment::value::de::DefaultInterpreter>::{closure#2}>> 0.0% 0.1% 64.0KiB foundry_config <::deserialize::__Visitor as serde::de::Visitor>::visit_map::::deserialize_from<::deserialize::__Visitor, figment::value::de::DefaultInterpreter>::{closure#2}>> 0.0% 0.1% 64.0KiB foundry_config <::deserialize::__Visitor as serde::de::Visitor>::visit_map::::deserialize_from<::deserialize::__Visitor, figment::value::de::DefaultInterpreter>::{closure#0}>> 0.0% 0.1% 64.0KiB foundry_config <::deserialize::__Visitor as serde::de::Visitor>::visit_map::::deserialize_from<::deserialize::__Visitor, figment::value::de::DefaultInterpreter>::{closure#0}>> ``` Essentially, each of `Tagged<()>`, `RelativePathBuf`, and `Value` have their own instantiation of `MapDe`, which duplicates the massive automatically-derived `serde` `visit_map` method. [`cargo-bloat`]: https://github.com/RazrFalcon/cargo-bloat This PR removes this `F` generic by introducing a separate trait, implemented on the deserializer `D`, that is just a simple constructor for `D`. Another approach would be simply converting the generic `F: Fn...` to `&dyn Fn`, but this introduces some runtime overhead, so I've opted for the slightly more verbose approach. --- src/value/de.rs | 76 +++++++++++++++++++++++++++++----------------- src/value/magic.rs | 11 +++---- 2 files changed, 52 insertions(+), 35 deletions(-) diff --git a/src/value/de.rs b/src/value/de.rs index b4ceb6f..3e0efa7 100644 --- a/src/value/de.rs +++ b/src/value/de.rs @@ -76,15 +76,14 @@ impl<'de: 'c, 'c, I: Interpreter> Deserializer<'de> for ConfiguredValueDe<'c, I> fn deserialize_any(self, v: V) -> Result where V: de::Visitor<'de> { - let maker = |v| Self::from(self.config, v); let result = match *self.value { Value::String(_, ref s) => v.visit_str(s), Value::Char(_, c) => v.visit_char(c), Value::Bool(_, b) => v.visit_bool(b), Value::Num(_, n) => n.deserialize_any(v), Value::Empty(_, e) => e.deserialize_any(v), - Value::Dict(_, ref map) => v.visit_map(MapDe::new(map, maker)), - Value::Array(_, ref seq) => v.visit_seq(SeqDe::new(seq, maker)), + Value::Dict(_, ref map) => v.visit_map(MapDe::::new(map, self.config)), + Value::Array(_, ref seq) => v.visit_seq(SeqDe::::new(seq, self.config)), }; result.map_err(|e| e.retagged(self.value.tag()).resolved(self.config)) @@ -134,8 +133,7 @@ impl<'de: 'c, 'c, I: Interpreter> Deserializer<'de> for ConfiguredValueDe<'c, I> let result = match self.value { Value::String(_, s) => v.visit_enum((&**s).into_deserializer()), Value::Dict(_, ref map) => { - let maker = |v| Self::from(self.config, v); - let map_access = MapDe::new(map, maker); + let map_access = MapDe::::new(map, self.config); v.visit_enum(MapAccessDeserializer::new(map_access)) } Value::Num(_, n) if n.to_u32().is_some() => { @@ -182,20 +180,42 @@ impl<'de: 'c, 'c, I: Interpreter> Deserializer<'de> for ConfiguredValueDe<'c, I> use std::collections::btree_map::Iter; -pub struct MapDe<'m, D, F: Fn(&'m Value) -> D> { - iter: Iter<'m, String, Value>, - pair: Option<(&'m String, &'m Value)>, - make_deserializer: F, +pub trait MakeDeserializer<'v> { + type Args: Copy; + + fn make_deserializer(value: &'v Value, args: Self::Args) -> Self; +} + +impl<'v, I: Interpreter> MakeDeserializer<'v> for ConfiguredValueDe<'v, I> { + type Args = &'v Figment; + + fn make_deserializer(value: &'v Value, config: &'v Figment) -> Self { + Self::from(config, value) + } +} + +impl<'v> MakeDeserializer<'v> for &'v Value { + type Args = (); + + fn make_deserializer(value: &'v Value, (): ()) -> Self { + value + } +} + +pub struct MapDe<'v, 'f, D: MakeDeserializer<'f>> { + iter: Iter<'v, String, Value>, + pair: Option<(&'v String, &'v Value)>, + args: D::Args, } -impl<'m, D, F: Fn(&'m Value) -> D> MapDe<'m, D, F> { - pub fn new(map: &'m Dict, maker: F) -> Self { - MapDe { iter: map.iter(), pair: None, make_deserializer: maker } +impl<'v, 'f, D: MakeDeserializer<'f>> MapDe<'v, 'f, D> { + pub fn new(map: &'v Dict, args: D::Args) -> Self { + MapDe { iter: map.iter(), pair: None, args } } } -impl<'m, 'de, D, F> de::MapAccess<'de> for MapDe<'m, D, F> - where D: Deserializer<'de, Error = Error>, F: Fn(&'m Value) -> D, +impl<'v: 'f, 'f, 'de, D> de::MapAccess<'de> for MapDe<'v, 'f, D> + where D: MakeDeserializer<'f> + Deserializer<'de, Error = Error>, { type Error = Error; @@ -219,35 +239,35 @@ impl<'m, 'de, D, F> de::MapAccess<'de> for MapDe<'m, D, F> { let (key, value) = self.pair.take().expect("visit_value called before visit_key"); let tag = value.tag(); - seed.deserialize((self.make_deserializer)(value)) + seed.deserialize(D::make_deserializer(value, self.args)) .map_err(|e: Error| e.prefixed(key).retagged(tag)) } } -pub struct SeqDe<'v, D, F: Fn(&'v Value) -> D> { +pub struct SeqDe<'v, 'f, D: MakeDeserializer<'f>> { iter: std::iter::Enumerate>, len: usize, - make_deserializer: F, + args: D::Args, } -impl<'v, D, F: Fn(&'v Value) -> D> SeqDe<'v, D, F> { - pub fn new(seq: &'v [Value], maker: F) -> Self { - SeqDe { len: seq.len(), iter: seq.iter().enumerate(), make_deserializer: maker } +impl<'v, 'f, D: MakeDeserializer<'f>> SeqDe<'v, 'f, D> { + pub fn new(seq: &'v [Value], args: D::Args) -> Self { + SeqDe { len: seq.len(), iter: seq.iter().enumerate(), args } } } -impl<'v, 'de, D, F> de::SeqAccess<'de> for SeqDe<'v, D, F> - where D: Deserializer<'de, Error = Error>, F: Fn(&'v Value) -> D, +impl<'v: 'f, 'f, 'de, D> de::SeqAccess<'de> for SeqDe<'v, 'f, D> + where D: MakeDeserializer<'f> + Deserializer<'de, Error = Error>, { type Error = Error; fn next_element_seed(&mut self, seed: T) -> Result> where T: de::DeserializeSeed<'de> { - if let Some((i, item)) = self.iter.next() { + if let Some((i, value)) = self.iter.next() { // item.map_tag(|metadata| metadata.path.push(self.count.to_string())); self.len -= 1; - seed.deserialize((self.make_deserializer)(item)) + seed.deserialize(D::make_deserializer(value, self.args)) .map_err(|e: Error| e.prefixed(&i.to_string())) .map(Some) } else { @@ -273,8 +293,8 @@ impl<'de> Deserializer<'de> for &Value { Bool(_, b) => v.visit_bool(b), Num(_, n) => n.deserialize_any(v), Empty(_, e) => e.deserialize_any(v), - Dict(_, ref map) => v.visit_map(MapDe::new(map, |v| v)), - Array(_, ref seq) => v.visit_seq(SeqDe::new(seq, |v| v)), + Dict(_, ref map) => v.visit_map(MapDe::<&Value>::new(map, ())), + Array(_, ref seq) => v.visit_seq(SeqDe::<&Value>::new(seq, ())), }; result.map_err(|e: Error| e.retagged(self.tag())) @@ -301,7 +321,7 @@ impl<'de> Deserializer<'de> for &Value { let result = match self { Value::String(_, s) => v.visit_enum((&**s).into_deserializer()), Value::Dict(_, ref map) => { - let map_access = MapDe::new(map, |v| v); + let map_access = MapDe::<&Value>::new(map, ()); v.visit_enum(MapAccessDeserializer::new(map_access)) } Value::Num(_, n) if n.to_u32().is_some() => { @@ -414,7 +434,7 @@ impl Value { let mut map = Dict::new(); map.insert(Self::FIELDS[0].into(), de.value.tag().into()); map.insert(Self::FIELDS[1].into(), de.value.clone()); - visitor.visit_map(MapDe::new(&map, |v| ConfiguredValueDe::<'_, I>::from(de.config, v))) + visitor.visit_map(MapDe::>::new(&map, de.config)) } } diff --git a/src/value/magic.rs b/src/value/magic.rs index ab37974..4b75bfc 100644 --- a/src/value/magic.rs +++ b/src/value/magic.rs @@ -186,8 +186,7 @@ impl Magic for RelativePathBuf { if let Some(d) = de.value.as_dict() { if let Some(mpv) = d.get(Self::FIELDS[0]) { if mpv.to_empty().is_none() { - let map_de = MapDe::new(d, |v| ConfiguredValueDe::::from(config, v)); - return visitor.visit_map(map_de); + return visitor.visit_map(MapDe::>::new(d, config)); } } } @@ -205,7 +204,7 @@ impl Magic for RelativePathBuf { // If we have this struct with no metadata_path, still use the value. let value = de.value.find_ref(Self::FIELDS[1]).unwrap_or(de.value); map.insert(Self::FIELDS[1].into(), value.clone()); - visitor.visit_map(MapDe::new(&map, |v| ConfiguredValueDe::::from(config, v))) + visitor.visit_map(MapDe::>::new(&map, config)) } } @@ -584,9 +583,7 @@ impl Deserialize<'de>> Magic for Tagged { if let Some(dict) = de.value.as_dict() { if let Some(tagv) = dict.get(Self::FIELDS[0]) { if let Ok(false) = tagv.deserialize::().map(|t| t.is_default()) { - return visitor.visit_map(MapDe::new(dict, |v| { - ConfiguredValueDe::::from(config, v) - })); + return visitor.visit_map(MapDe::>::new(dict, config)); } } } @@ -595,7 +592,7 @@ impl Deserialize<'de>> Magic for Tagged { let value = de.value.find_ref(Self::FIELDS[1]).unwrap_or(de.value); map.insert(Self::FIELDS[0].into(), de.value.tag().into()); map.insert(Self::FIELDS[1].into(), value.clone()); - visitor.visit_map(MapDe::new(&map, |v| ConfiguredValueDe::::from(config, v))) + visitor.visit_map(MapDe::>::new(&map, config)) } }