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

Improve compile time #211

Open
Emilgardis opened this issue Sep 11, 2021 · 10 comments
Open

Improve compile time #211

Emilgardis opened this issue Sep 11, 2021 · 10 comments
Labels
help wanted Extra attention is needed

Comments

@Emilgardis
Copy link
Member

Compilation with some features makes the crate take ages to compile. Need to figure out why, and try to mitigate it.

Experiments need to be done to figure out what is taking all the time, I suspect codegen is a big timewaste.

Related to #203 and #204

@Emilgardis Emilgardis added the help wanted Extra attention is needed label Sep 11, 2021
@Emilgardis Emilgardis pinned this issue Sep 11, 2021
@Emilgardis
Copy link
Member Author

These tests should be done on windows and linux

@Emilgardis
Copy link
Member Author

Should try to make a reproducible benchmark to keep track of this, I feel like the compile time has improved with rustc versions

@Emilgardis
Copy link
Member Author

Emilgardis commented Mar 6, 2023

❯ summarize summarize --percent-above 10 twitch_api-0025240.mm_profdata
+------------------------------+-----------+-----------------+--------+------------+---------------------------------+
| Item                         | Self time | % of total time | Time   | Item count | Incremental result hashing time |
+------------------------------+-----------+-----------------+--------+------------+---------------------------------+
| LLVM_module_codegen_emit_obj | 59.75s    | 53.079          | 59.75s | 256        | 0.00ns                          |
+------------------------------+-----------+-----------------+--------+------------+---------------------------------+
| LLVM_passes                  | 15.30s    | 13.594          | 15.30s | 1          | 0.00ns                          |
+------------------------------+-----------+-----------------+--------+------------+---------------------------------+
| codegen_module               | 11.34s    | 10.076          | 14.73s | 256        | 0.00ns                          |
+------------------------------+-----------+-----------------+--------+------------+---------------------------------+
Total cpu time: 112.558478793s
Filtered results account for 76.749% of total time.
+----------------------------+-----------------+
| Item                       | Artifact Size   |
+----------------------------+-----------------+
| codegen_unit_size_estimate | 3781590 bytes   |
+----------------------------+-----------------+
| crate_metadata             | 53295483 bytes  |
+----------------------------+-----------------+
| dep_graph                  | 158831053 bytes |
+----------------------------+-----------------+
| linked_artifact            | 323684368 bytes |
+----------------------------+-----------------+
| object_file                | 251006424 bytes |
+----------------------------+-----------------+
| query_cache                | 93758624 bytes  |
+----------------------------+-----------------+
| work_product_index         | 14794 bytes     |

cargo llvm-lines --all-features

  Lines                  Copies                Function name
  -----                  ------                -------------
  7150872                136439                (TOTAL)
   715551 (10.0%, 10.0%)  17022 (12.5%, 12.5%) core::result::Result<T,E>::map_err
   544038 (7.6%, 17.6%)    2349 (1.7%, 14.2%)  <serde_json::de::SeqAccess<R> as serde::de::SeqAccess>::next_element_seed
   461914 (6.5%, 24.1%)     864 (0.6%, 14.8%)  <&mut serde_json::de::Deserializer<R> as serde::de::Deserializer>::deserialize_struct
   326040 (4.6%, 28.6%)     247 (0.2%, 15.0%)  <twitch_api::eventsub::event::_::<impl serde::de::Deserialize for twitch_api::eventsub::event::EventType>::deserialize::__Visitor as serde::de::Visitor>::visit_enum
   261818 (3.7%, 32.3%)    2405 (1.8%, 16.8%)  <serde_path_to_error::de::MapAccess<X> as serde::de::MapAccess>::next_value_seed
   226709 (3.2%, 35.5%)     316 (0.2%, 17.0%)  <&mut serde_json::de::Deserializer<R> as serde::de::Deserializer>::deserialize_any
   213487 (3.0%, 38.5%)    2349 (1.7%, 18.7%)  <serde_path_to_error::de::SeqAccess<X> as serde::de::SeqAccess>::next_element_seed
   208653 (2.9%, 41.4%)    2496 (1.8%, 20.6%)  <serde_path_to_error::de::TrackedSeed<X> as serde::de::DeserializeSeed>::deserialize
   196638 (2.7%, 44.1%)    2112 (1.5%, 22.1%)  <serde_ignored::MapAccess<X,F> as serde::de::MapAccess>::next_value_seed
   156592 (2.2%, 46.3%)     160 (0.1%, 22.2%)  <twitch_api::eventsub::_::<impl serde::de::Deserialize for twitch_api::eventsub::EventSubscriptionInformation<E>>::deserialize::__Visitor<E> as serde::de::Visitor>::visit_map
   144270 (2.0%, 48.3%)    2403 (1.8%, 24.0%)  <serde_json::de::MapAccess<R> as serde::de::MapAccess>::next_value_seed
   143722 (2.0%, 50.3%)    2053 (1.5%, 25.5%)  <serde_ignored::SeqAccess<X,F> as serde::de::SeqAccess>::next_element_seed
   128832 (1.8%, 52.1%)     244 (0.2%, 25.7%)  <twitch_api::eventsub::_::<impl serde::de::Deserialize for twitch_api::eventsub::Status>::deserialize::__Visitor as serde::de::Visitor>::visit_enum
    87906 (1.2%, 53.4%)     289 (0.2%, 25.9%)  <&mut serde_json::de::Deserializer<R> as serde::de::Deserializer>::deserialize_enum
    76217 (1.1%, 54.4%)     364 (0.3%, 26.1%)  <serde_json::de::MapAccess<R> as serde::de::MapAccess>::next_key_seed
    75925 (1.1%, 55.5%)    1247 (0.9%, 27.1%)  serde_path_to_error::de::<impl serde::de::Visitor for serde_path_to_error::wrap::Wrap<X>>::visit_seq
    73786 (1.0%, 56.5%)    1212 (0.9%, 27.9%)  serde_path_to_error::de::<impl serde::de::Visitor for serde_path_to_error::wrap::Wrap<X>>::visit_map
    70721 (1.0%, 57.5%)     864 (0.6%, 28.6%)  <serde_path_to_error::de::Deserializer<D> as serde::de::Deserializer>::deserialize_struct
    67832 (0.9%, 58.5%)     160 (0.1%, 28.7%)  <twitch_api::eventsub::_::<impl serde::de::Deserialize for twitch_api::eventsub::EventSubscriptionInformation<E>>::deserialize::__Visitor<E> as serde::de::Visitor>::visit_seq
    61713 (0.9%, 59.3%)     332 (0.2%, 28.9%)  <&mut serde_json::de::Deserializer<R> as serde::de::Deserializer>::deserialize_str
    45126 (0.6%, 60.0%)    2232 (1.6%, 30.6%)  serde_path_to_error::wrap::Wrap<X>::new
    41007 (0.6%, 60.5%)    1005 (0.7%, 31.3%)  core::result::Result<T,E>::map

A lot of the time comes from llvm, and most of it seems to be serde, I wonder if there's a neat way to avoid this, perhaps making json the only supported serialization/deserialization unless something else is wanted, thus skipping serde

@Emilgardis
Copy link
Member Author

Maybe miniserde or serde-lite could help?

miniserde doesn't have all the features we're using currently but serde-lite could maybe be interesting, but doesn't either have all the features we use

@Emilgardis
Copy link
Member Author

Emilgardis commented Aug 27, 2023

Removing a single module in helix makes quite a difference, I wonder if there's something bad we're doing that can be fixed by forcing the compiler to not inline or to actually inline

@Nerixyz
Copy link
Contributor

Nerixyz commented Aug 27, 2023

Removing a single module in helix makes quite a difference

Maybe modules could be en-/disabled one-by-one? I think it's rarely the case that you need every helix/eventsub/... module.

@Emilgardis
Copy link
Member Author

Maybe modules could be en-/disabled one-by-one? I think it's rarely the case that you need every helix/eventsub/... module.

That sounds very easy, could possibly have multiple scopes, and with the new feature where cargo tell you that an item is behind a cfg, quite discoverable

- bits
- channels
- charity
- chat
- client
- clips
- eventsub
- games
- goals
- hypetrain
- moderation
- points
- polls
- predictions
- raids
- request
- response
- request
- schedule
- search
- streams
- subscriptions
- tags
- teams
- users
- videos
- whispers

that's a lot of feature flags though :3

@simonsan
Copy link

"Take serde. If one package enables the derive feature then packages that don't need it, like serde_json or toml, can't build until all of the proc-macro machinery is built."

ref.: https://www.reddit.com/r/rust/comments/1602eah/comment/jxkciap/?utm_source=reddit&utm_medium=web2x&context=3

So you could also split out the derive feature and pull in serde_derive extra, as a start.

@Emilgardis
Copy link
Member Author

Emilgardis commented Aug 27, 2023

the problem is not serde or serde_derive, the problem is the code it generates @simonsan, but we could definitely switch to using serde_derive to make compilation more parallel. Related serde-rs/serde#2584 (comment)

edit: having trouble decoupling serde/derive and serde_derive in twitch_types due to needing serde?/derive

@Emilgardis
Copy link
Member Author

Emilgardis commented Nov 21, 2023

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

3 participants