Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reduce monomorphization in Deserializer implementation #126

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

DaniPopes
Copy link

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:

...
 0.0%   0.1%  64.2KiB                    foundry_config <<foundry_config::Config as serde::de::Deserialize>::deserialize::__Visitor as serde::de::Visitor>::visit_map::<figment::value::de::MapDe<figment::value::de::ConfiguredValueDe, <figment::value::magic::Tagged<()> as figment::value::magic::Magic>::deserialize_from<<foundry_config::Config as serde::de::Deserialize>::deserialize::__Visitor, figment::value::de::DefaultInterpreter>::{closure#1}>>
 0.0%   0.1%  64.2KiB                    foundry_config <<foundry_config::Config as serde::de::Deserialize>::deserialize::__Visitor as serde::de::Visitor>::visit_map::<figment::value::de::MapDe<figment::value::de::ConfiguredValueDe, <figment::value::magic::Tagged<()> as figment::value::magic::Magic>::deserialize_from<<foundry_config::Config as serde::de::Deserialize>::deserialize::__Visitor, figment::value::de::DefaultInterpreter>::{closure#2}>>
 0.0%   0.1%  64.2KiB                    foundry_config <<foundry_config::Config as serde::de::Deserialize>::deserialize::__Visitor as serde::de::Visitor>::visit_map::<figment::value::de::MapDe<figment::value::de::ConfiguredValueDe, <figment::value::magic::RelativePathBuf as figment::value::magic::Magic>::deserialize_from<<foundry_config::Config as serde::de::Deserialize>::deserialize::__Visitor, figment::value::de::DefaultInterpreter>::{closure#2}>>
 0.0%   0.1%  64.2KiB                    foundry_config <<foundry_config::Config as serde::de::Deserialize>::deserialize::__Visitor as serde::de::Visitor>::visit_map::<figment::value::de::MapDe<figment::value::de::ConfiguredValueDe, <figment::value::magic::RelativePathBuf as figment::value::magic::Magic>::deserialize_from<<foundry_config::Config as serde::de::Deserialize>::deserialize::__Visitor, figment::value::de::DefaultInterpreter>::{closure#0}>>
 0.0%   0.1%  64.2KiB                    foundry_config <<foundry_config::Config as serde::de::Deserialize>::deserialize::__Visitor as serde::de::Visitor>::visit_map::<figment::value::de::MapDe<figment::value::de::ConfiguredValueDe, <figment::value::value::Value>::deserialize_from<<foundry_config::Config as serde::de::Deserialize>::deserialize::__Visitor, figment::value::de::DefaultInterpreter>::{closure#0}>>
 0.0%   0.1%  64.0KiB                    foundry_config <<foundry_config::Config as serde::de::Deserialize>::deserialize::__Visitor as serde::de::Visitor>::visit_map::<figment::value::de::MapDe<figment::value::de::ConfiguredValueDe, <figment::value::magic::Tagged<()> as figment::value::magic::Magic>::deserialize_from<<foundry_config::Config as serde::de::Deserialize>::deserialize::__Visitor, figment::value::de::DefaultInterpreter>::{closure#1}>>
 0.0%   0.1%  64.0KiB                    foundry_config <<foundry_config::Config as serde::de::Deserialize>::deserialize::__Visitor as serde::de::Visitor>::visit_map::<figment::value::de::MapDe<figment::value::de::ConfiguredValueDe, <figment::value::magic::Tagged<()> as figment::value::magic::Magic>::deserialize_from<<foundry_config::Config as serde::de::Deserialize>::deserialize::__Visitor, figment::value::de::DefaultInterpreter>::{closure#2}>>
 0.0%   0.1%  64.0KiB                    foundry_config <<foundry_config::Config as serde::de::Deserialize>::deserialize::__Visitor as serde::de::Visitor>::visit_map::<figment::value::de::MapDe<figment::value::de::ConfiguredValueDe, <figment::value::magic::RelativePathBuf as figment::value::magic::Magic>::deserialize_from<<foundry_config::Config as serde::de::Deserialize>::deserialize::__Visitor, figment::value::de::DefaultInterpreter>::{closure#2}>>
 0.0%   0.1%  64.0KiB                    foundry_config <<foundry_config::Config as serde::de::Deserialize>::deserialize::__Visitor as serde::de::Visitor>::visit_map::<figment::value::de::MapDe<figment::value::de::ConfiguredValueDe, <figment::value::magic::RelativePathBuf as figment::value::magic::Magic>::deserialize_from<<foundry_config::Config as serde::de::Deserialize>::deserialize::__Visitor, figment::value::de::DefaultInterpreter>::{closure#0}>>
 0.0%   0.1%  64.0KiB                    foundry_config <<foundry_config::Config as serde::de::Deserialize>::deserialize::__Visitor as serde::de::Visitor>::visit_map::<figment::value::de::MapDe<figment::value::de::ConfiguredValueDe, <figment::value::value::Value>::deserialize_from<<foundry_config::Config as serde::de::Deserialize>::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.

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.

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 <<foundry_config::Config as serde::de::Deserialize>::deserialize::__Visitor as serde::de::Visitor>::visit_map::<figment::value::de::MapDe<figment::value::de::ConfiguredValueDe, <figment::value::magic::Tagged<()> as figment::value::magic::Magic>::deserialize_from<<foundry_config::Config as serde::de::Deserialize>::deserialize::__Visitor, figment::value::de::DefaultInterpreter>::{closure#1}>>
 0.0%   0.1%  64.2KiB                    foundry_config <<foundry_config::Config as serde::de::Deserialize>::deserialize::__Visitor as serde::de::Visitor>::visit_map::<figment::value::de::MapDe<figment::value::de::ConfiguredValueDe, <figment::value::magic::Tagged<()> as figment::value::magic::Magic>::deserialize_from<<foundry_config::Config as serde::de::Deserialize>::deserialize::__Visitor, figment::value::de::DefaultInterpreter>::{closure#2}>>
 0.0%   0.1%  64.2KiB                    foundry_config <<foundry_config::Config as serde::de::Deserialize>::deserialize::__Visitor as serde::de::Visitor>::visit_map::<figment::value::de::MapDe<figment::value::de::ConfiguredValueDe, <figment::value::magic::RelativePathBuf as figment::value::magic::Magic>::deserialize_from<<foundry_config::Config as serde::de::Deserialize>::deserialize::__Visitor, figment::value::de::DefaultInterpreter>::{closure#2}>>
 0.0%   0.1%  64.2KiB                    foundry_config <<foundry_config::Config as serde::de::Deserialize>::deserialize::__Visitor as serde::de::Visitor>::visit_map::<figment::value::de::MapDe<figment::value::de::ConfiguredValueDe, <figment::value::magic::RelativePathBuf as figment::value::magic::Magic>::deserialize_from<<foundry_config::Config as serde::de::Deserialize>::deserialize::__Visitor, figment::value::de::DefaultInterpreter>::{closure#0}>>
 0.0%   0.1%  64.2KiB                    foundry_config <<foundry_config::Config as serde::de::Deserialize>::deserialize::__Visitor as serde::de::Visitor>::visit_map::<figment::value::de::MapDe<figment::value::de::ConfiguredValueDe, <figment::value::value::Value>::deserialize_from<<foundry_config::Config as serde::de::Deserialize>::deserialize::__Visitor, figment::value::de::DefaultInterpreter>::{closure#0}>>
 0.0%   0.1%  64.0KiB                    foundry_config <<foundry_config::Config as serde::de::Deserialize>::deserialize::__Visitor as serde::de::Visitor>::visit_map::<figment::value::de::MapDe<figment::value::de::ConfiguredValueDe, <figment::value::magic::Tagged<()> as figment::value::magic::Magic>::deserialize_from<<foundry_config::Config as serde::de::Deserialize>::deserialize::__Visitor, figment::value::de::DefaultInterpreter>::{closure#1}>>
 0.0%   0.1%  64.0KiB                    foundry_config <<foundry_config::Config as serde::de::Deserialize>::deserialize::__Visitor as serde::de::Visitor>::visit_map::<figment::value::de::MapDe<figment::value::de::ConfiguredValueDe, <figment::value::magic::Tagged<()> as figment::value::magic::Magic>::deserialize_from<<foundry_config::Config as serde::de::Deserialize>::deserialize::__Visitor, figment::value::de::DefaultInterpreter>::{closure#2}>>
 0.0%   0.1%  64.0KiB                    foundry_config <<foundry_config::Config as serde::de::Deserialize>::deserialize::__Visitor as serde::de::Visitor>::visit_map::<figment::value::de::MapDe<figment::value::de::ConfiguredValueDe, <figment::value::magic::RelativePathBuf as figment::value::magic::Magic>::deserialize_from<<foundry_config::Config as serde::de::Deserialize>::deserialize::__Visitor, figment::value::de::DefaultInterpreter>::{closure#2}>>
 0.0%   0.1%  64.0KiB                    foundry_config <<foundry_config::Config as serde::de::Deserialize>::deserialize::__Visitor as serde::de::Visitor>::visit_map::<figment::value::de::MapDe<figment::value::de::ConfiguredValueDe, <figment::value::magic::RelativePathBuf as figment::value::magic::Magic>::deserialize_from<<foundry_config::Config as serde::de::Deserialize>::deserialize::__Visitor, figment::value::de::DefaultInterpreter>::{closure#0}>>
 0.0%   0.1%  64.0KiB                    foundry_config <<foundry_config::Config as serde::de::Deserialize>::deserialize::__Visitor as serde::de::Visitor>::visit_map::<figment::value::de::MapDe<figment::value::de::ConfiguredValueDe, <figment::value::value::Value>::deserialize_from<<foundry_config::Config as serde::de::Deserialize>::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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant