make some of the code behind build_files non-polymorphic

[srijs]

Hi, thanks so much for putting together this crate, it's been super helpful in building dynamic gRPC functionality.

I'm overall very happy with prost-reflect, but while working on reducing the size of my binaries using cargo bloat I noticed that it was using significant space in the text section, both directly through decode_file_descriptor_set and decode but also in the methods in my code calling decode_file_descriptor_set due to inlining.

Trying to understand why this might be, I came across this optimization. The problem with the build_files method is that it takes a generic IntoIterator, which means the function is monomorphized at multiple sites in DescriptorPool. Because the logic inside build_files becomes quite complex with the different visitors, this results in a decent amount of code bloat.

The change I'm proposing here simply moves the bulk of the heavy logic into a non-polymorphic function (build_files_deduped) without changing any functionality whatsoever. This should be fully equivalent in terms of performance and behaviour, with the only difference being that the binary footprint of users of the crate should be significantly reduced.

In my particular case, this change reduces the total binary size by ~86KiB, which represents a saving of more than 1/4 of the total code size attributable to prost-reflect (335.4KiB according to cargo bloat before the optimization).

---

As an aside, I've also noticed that there are two other places where generics cause multiple copies of very large functions and/or sections of code: Visitor and FieldDescriptorLike. I am wondering whether you have considered switching the functions taking impls of these traits to use dyn traits instead? That seems like it has the potential to reduce code size somewhat further, but may need to be benchmarked due to the dynamic dispatch it'd introduce.

[codecov[bot]]

Codecov Report

Patch coverage: 100.00% and project coverage change: -0.28 :warning:

Comparison is base (5777b55) 75.63% compared to head (0dffcd0) 75.36%.

Additional details and impacted files

```diff @@ Coverage Diff @@ ## main #40 +/- ## ========================================== - Coverage 75.63% 75.36% -0.28% ========================================== Files 31 31 Lines 5254 5253 -1 ========================================== - Hits 3974 3959 -15 - Misses 1280 1294 +14 ``` | [Impacted Files](https://codecov.io/gh/andrewhickman/prost-reflect/pull/40?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Andrew+Hickman) | Coverage Δ | | |---|---|---| | [prost-reflect/src/descriptor/build/mod.rs](https://codecov.io/gh/andrewhickman/prost-reflect/pull/40?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Andrew+Hickman#diff-cHJvc3QtcmVmbGVjdC9zcmMvZGVzY3JpcHRvci9idWlsZC9tb2QucnM=) | `98.78% <100.00%> (ø)` | | | [prost-reflect/src/descriptor/build/names.rs](https://codecov.io/gh/andrewhickman/prost-reflect/pull/40?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Andrew+Hickman#diff-cHJvc3QtcmVmbGVjdC9zcmMvZGVzY3JpcHRvci9idWlsZC9uYW1lcy5ycw==) | `65.68% <100.00%> (-0.99%)` | :arrow_down: | | [prost-reflect/src/descriptor/build/options.rs](https://codecov.io/gh/andrewhickman/prost-reflect/pull/40?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Andrew+Hickman#diff-cHJvc3QtcmVmbGVjdC9zcmMvZGVzY3JpcHRvci9idWlsZC9vcHRpb25zLnJz) | `62.22% <100.00%> (-0.25%)` | :arrow_down: | | [prost-reflect/src/descriptor/build/resolve.rs](https://codecov.io/gh/andrewhickman/prost-reflect/pull/40?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Andrew+Hickman#diff-cHJvc3QtcmVmbGVjdC9zcmMvZGVzY3JpcHRvci9idWlsZC9yZXNvbHZlLnJz) | `66.38% <100.00%> (-1.03%)` | :arrow_down: | | [prost-reflect/src/descriptor/build/visit.rs](https://codecov.io/gh/andrewhickman/prost-reflect/pull/40?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Andrew+Hickman#diff-cHJvc3QtcmVmbGVjdC9zcmMvZGVzY3JpcHRvci9idWlsZC92aXNpdC5ycw==) | `100.00% <100.00%> (ø)` | | ... and [5 files with indirect coverage changes](https://codecov.io/gh/andrewhickman/prost-reflect/pull/40/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Andrew+Hickman) Help us with your feedback. Take ten seconds to tell us [how you rate us](https://about.codecov.io/nps?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Andrew+Hickman). Have a feature suggestion? [Share it here.](https://app.codecov.io/gh/feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Andrew+Hickman)

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

[andrewhickman]

Thanks for the PR!

As an aside, I've also noticed that there are two other places where generics cause multiple copies of very large functions and/or sections of code: Visitor and FieldDescriptorLike. I am wondering whether you have considered switching the functions taking impls of these traits to use dyn traits instead? That seems like it has the potential to reduce code size somewhat further, but may need to be benchmarked due to the dynamic dispatch it'd introduce.

I think switching Visitor to dynamic dispatch should be fine, I don't expect building a descriptor pool to be a performance-critical operation.

FieldDescriptorLike is used in a lot of serialization/deserialization code where performance is a bit more important. We could potentially split out the common fields of FieldDescriptor and ExtensionDescriptor into a common struct that both codepaths can reference. Alternatively a simple enum of FieldDescriptor and ExtensionDescriptor might be worth benchmarking