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

[nfc] Clean up recursive header dependencies #2039

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

Conversation

fhanau
Copy link
Collaborator

@fhanau fhanau commented May 14, 2024

This should slightly improve compile speeds, especially under MSVC. To keep this change small this primarily affects headers, which should already allow us to eliminate many superfluous recursive includes.

  • Limit usage of intrin.h for MSVC, this is generally heavy
  • Drop some support for MSVC versions older than VS 2017 Update 9 (MSVC_VER 1916). We likely don't support this version anyway based on coroutines/C++20 support, but dropping more _MSC_VER checks can be done in another PR.

Further potential to reduce recursive includes exists by refactoring async code so that coroutines are not always included, as well as isolating usage of large libc++ headers like <set> and <atomic>. I experimented with those and based on looking at preprocessed source code sizes in a more rigorous way this leads to larger improvements in include sizes, but the advantage in compile time is harder to evaluate.

This will likely require some include changes downstream which are still Work-In-Progress, but this should be final and ready for review.

@fhanau fhanau force-pushed the felix/header-cleanup branch 3 times, most recently from 400221f to 8e78ad4 Compare May 18, 2024 06:13
@@ -5,9 +5,6 @@

#include <capnp/generated-header-support.h>
#include <kj/windows-sanity.h>
#if !CAPNP_LITE
Copy link
Collaborator Author

@fhanau fhanau May 26, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Regenerated the header, the extra include directives are no longer being generated.

This should slightly improve compile speeds, especially under MSVC. To
keep this change small this primarily affects headers, which should
already allow us to eliminate many superfluous recursive includes.

- Limit usage of intrin.h for MSVC which is generally large, describe
  what it is used for.
- Drop some support for MSVC versions older than VS 2017 Update 9
  (_MSC_VER 1916).
@fhanau fhanau changed the title [DRAFT][nfc] Clean up recursive header dependencies [nfc] Clean up recursive header dependencies May 26, 2024
@fhanau fhanau marked this pull request as ready for review May 26, 2024 17:07
@fhanau fhanau requested a review from mikea May 26, 2024 17:07
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.

None yet

1 participant