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 times by 25% #149

Merged
merged 12 commits into from
Dec 19, 2024
Merged

Improve compile times by 25% #149

merged 12 commits into from
Dec 19, 2024

Conversation

jeaye
Copy link
Member

@jeaye jeaye commented Dec 19, 2024

This drops (serial) compile times from ~980s to ~740 seconds. This is important not just for our dev experience, but because we'll ultimately be including more of these header during runtime and startup. We need them to be as light as possible. I used clang build analyzer for figuring out the most important things to tackle. There's a lot more which can be done (we can definitely get under 500s), but for now this PR includes:

  • Replace static_object with distinct object types
  • Remove magic_enum in favor of constexpr fns
  • Move various CRTP templates into cpp files with explicit instantiations
  • Remove fmt from headers and provide a custom string builder instead
    • fmt is still used within cpp files (for now)
  • Clean up lots of includes based on all of the changes above

jeaye added 12 commits December 13, 2024 15:45
Instead, each object type is its own standalone struct. This allows for
easier forward declaration of object types, which should allow for
faster compile times as we continue to strip down our header
dependencies.

It also removes a couple more templates, which will hopefully just make
things simpler to grok for newcomers.
It slows compilation down a ton. By using clang time tracing, I found
we're spending over 100 seconds during compilation just getting enum names.
We know all of the instantiations, so we don't need the implementation
in the headers.
This cuts clang compilation parse time down from 800s to 700s.
@jeaye jeaye merged commit c88df7b into main Dec 19, 2024
5 checks passed
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