-
-
Notifications
You must be signed in to change notification settings - Fork 380
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
Refactor Header #include Knots #5309
Refactor Header #include Knots #5309
Conversation
And yes, I borked the MSVC build myself, need to refactor the name of the new low-level math file... |
1627f83
to
9195820
Compare
Update: At this point I think this PR is ready for merge (once MSVC issues have been resolved). Feel free to review the PR, but there's very little substance to the changes other than reordering/refactoring EDIT: I will not be resolving the clang-format failures, as they're mostly spurious whitespace issues not introduced by this PR. |
876b6fc
to
b5bc47c
Compare
90e4530
to
6a3a044
Compare
acff4b5
to
afbbbcb
Compare
I've updated this PR to current master and made further improvements. All numbers reported are compiled on a Ryzen 7 5800X. Current master (68s wall-clock time):
Final result (46s wall-clock time):
Most of the codegen time improvement (~100s) came from removing the In total, this PR represents a 33% compilation time savings on my machine when compiled with 16 workers. The effect on CI builds will most likely be less, as they're comparatively less disk-bound and more cpu-bound. |
Additionally, it looks like CI compilation time went from ~16m to ~11m for Clang, and ~23m to ~16m for GCC. |
Seriously optimize our usage of large headers with significant compile time penalties. - Removed SDL headers from libs.h (significant savings from this alone). - Used forward-declares for Input and Pigui in GuiApplication - Removed many uses of libs.h from header files that were being chain-included. - Removed all uses of Json.h from header files; JsonUtils remains for use in CPP files - Removed several unnecessary uses of libs.h from cpp files. This has significantly reduced the time cost involved in parsing libs.h and reduced the overall compile time of the project. Most cpp files as a consequence have shifted towards an include-what-you-use structure, often with only one or two includes required to replace libs.h.
- Saves ~30% overall codegen time across the entire project. - Avoids instantiating fmt::vsprintf in every compilation unit that uses Output etc.
- Most uses of utils.h were just for logging functionality - String formatting functions are in core/StringUtils.h - Double-to-string munging functions moved to JsonUtils pending removal - isqrt etc. were moved to MathUtil - NOMINMAX is globally defined when compiling under MSVC
- utils.h now only includes a few core headers - Generally reduce the usage of utils.h and replace with Log.h and MathUtil.h as needed
The peak of the great de-libbening; most files are now include-what-you-use.
193fc0a
to
418c574
Compare
- Fix MSVC build missing <cinttypes> - Fix MSVC missing strncasecmp - Ensure cmath is included for MSVC builds
0f0318c
to
a2d9fb0
Compare
Nice! |
This PR lowers compilation time by 15-20% on my development system by simply re-arranging our include files and not relying so heavily on the monolithic
libs.h
. As a result, the amount of time spent just textually including and parsing libs.h in a full rebuild has dropped by by over 75%.