Skip to content

[CI] Improved Compatibility Build Speed #3184

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

Merged
merged 3 commits into from
Jul 4, 2025

Conversation

AlexandreSinger
Copy link
Contributor

The compatibility builds do not run any tests. As such, they do not need to use compiler optimizations or IPO. By turning these features off, we should be able to speed up the compilation process on these builds.

The compatibility builds do not run any tests. As such, they do not need
to use compiler optimizations or IPO. By turning these features off, we
should be able to speed up the compilation process on these builds.
@github-actions github-actions bot added infra Project Infrastructure lang-shell Shell scripts (bash etc.) labels Jul 3, 2025
@AlexandreSinger AlexandreSinger force-pushed the feature-ci-compat-speedup branch from e6cac71 to e76960d Compare July 3, 2025 23:23
@AmirhosseinPoolad
Copy link
Contributor

I only checked 'B: GCC 11 (Ubuntu Noble - 24.04' but it appears to be around 4 minutes faster. Good change! Since we're not using the debug symbols in any way, can we can get a tiny bit more speed here by not generating debug symbols at all? Something like this:

cmake -DCMAKE_BUILD_TYPE=Debug \
      -DCMAKE_C_FLAGS_DEBUG="-O0" \
      -DCMAKE_CXX_FLAGS_DEBUG="-O0" \

By default cmake also adds the -g flag for the Debug build type.

You should also add a comment which says that these builds are compiled with no optimizations and are used only for checking compatibility. Otherwise I feel like someone in the future might accidentally copy paste the config here for another set of tests that should be compiled with release configuration.

@AlexandreSinger
Copy link
Contributor Author

Hi @AmirhosseinPoolad , thanks for the feedback! I agree, this should be commented for the exact reason you give.

I would actually like to keep the symbols on in this case. This should make debugging a little easier and symbols do not add too much run time. But it is something we can investigate later. For now, the lowest hanging fruit is making this debug.

@AlexandreSinger
Copy link
Contributor Author

Also, FYI. The build times have been very unstable due to how we have been using the CCache action. It turns out we can make these much much more stable. I am working on a much larger PR which should cut the CI time down by at least 30 mins per run.

@AmirhosseinPoolad
Copy link
Contributor

I would actually like to keep the symbols on in this case. This should make debugging a little easier and symbols do not add too much run time.

I do agree that this is probably not very impactful runtime-wise but how would it make debugging easier or harder? Aren't these builds basically thrown away after they are compiled and we only care if the compilation succeeded or not?

@AlexandreSinger
Copy link
Contributor Author

I would actually like to keep the symbols on in this case. This should make debugging a little easier and symbols do not add too much run time.

I do agree that this is probably not very impactful runtime-wise but how would it make debugging easier or harder? Aren't these builds basically thrown away after they are compiled and we only care if the compilation succeeded or not?

Thats a good point. I was not thinking. But I have a feeling that someone may want to add "basic_tests" to these builds in the future. I feel like future proofing this since the cost of having debug symbols is so small.

@AlexandreSinger AlexandreSinger merged commit 589514f into master Jul 4, 2025
33 checks passed
@AlexandreSinger AlexandreSinger deleted the feature-ci-compat-speedup branch July 4, 2025 21:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
infra Project Infrastructure lang-shell Shell scripts (bash etc.)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants