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

Added ccache speedup compilation #7105

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

GermanAizek
Copy link

@GermanAizek GermanAizek commented Jan 30, 2024

Description

More effective helped after compilation checkout branches.
More information on https://ccache.dev

@willcode
Copy link
Member

Please sign your commit (git commit -S --amend) and force push, to acknowledge the DCO (contributor agreement).

@willcode
Copy link
Member

What kind of speedup did you see? Was there any speedup on an initial build, or only on subsequent builds?

@GermanAizek
Copy link
Author

What kind of speedup did you see? Was there any speedup on an initial build, or only on subsequent builds?

Yep, more effective recompilation after checkout branches.

@willcode
Copy link
Member

How much speedup?

Our CI does not have ccache installed, based on the cmake output in the logs. It wouldn't have any effect on CI anyway since that's a single compilation.

@GermanAizek
Copy link
Author

How much speedup?

Our CI does not have ccache installed, based on the cmake output in the logs. It wouldn't have any effect on CI anyway since that's a single compilation.

You're right, it doesn't apply to CI, it's convenience for developers.

@marcusmueller
Copy link
Member

marcusmueller commented Jan 30, 2024

on most Linux distros where ccache is installed, it does get used automatically:

git clone https://github.com/gnuradio/gnuradio
mkdir gnuradio/build-defaultconfig
cd gnuradio/build-defaultconfig
cmake ..
grep CMAKE_CXX_COMPILER CMakeCache.txt

yields

CMAKE_CXX_COMPILER:FILEPATH=/usr/lib64/ccache/c++
CMAKE_CXX_COMPILER_AR:FILEPATH=/usr/bin/gcc-ar
CMAKE_CXX_COMPILER_RANLIB:FILEPATH=/usr/bin/gcc-ranlib
//ADVANCED property for variable: CMAKE_CXX_COMPILER
CMAKE_CXX_COMPILER-ADVANCED:INTERNAL=1
//ADVANCED property for variable: CMAKE_CXX_COMPILER_AR
CMAKE_CXX_COMPILER_AR-ADVANCED:INTERNAL=1
//ADVANCED property for variable: CMAKE_CXX_COMPILER_RANLIB
CMAKE_CXX_COMPILER_RANLIB-ADVANCED:INTERNAL=1

without any further ado (on Fedora).

You also want to have a definitive way to disable it, as well. I'm not sure on which systems this is actually needed, so I'm kind of sceptical it's generally a good idea; we'd really need a killswitch to disable it in the face of compiler and linker trouble, or when you just don't want to use ccache, because you know this is going to be a modified one-off build.

@marcusmueller
Copy link
Member

By the way, @willcode:

Our CI does not have ccache installed, based on the cmake output in the logs. It wouldn't have any effect on CI anyway since that's a single compilation.

Definitely on my to-do list, we could (after adding ccache to the CI builders) have a CI run that we execute once per push to maint-3.10 or main, which then gets pushed as artifact; all the regular CI runs would first download and extract the ccache before running the build.

@willcode
Copy link
Member

I've seen it get picked up automatically in the past, too. @GermanAizek ccache is not used automatically on your distro? Which one is that?

I actually stopped using ccache when SSDs first came out and were pretty small, since the cache was always using up a lot of space. Haven't tried it again recently.

@mbr0wn
Copy link
Member

mbr0wn commented Feb 4, 2024

I have been also using ccache for almost a decade now with no CMake mods required. I'm not sure how this changeset will work for cases where ccache is available, but not desired (currently, you can simply do CXX=/usr/bin/g++ cmake .....). @GermanAizek what's your dev setup? Fedora/Ubuntu users certainly won't need this, but what system are you on? Maybe we're making things unnecessarily complicated for some OSs.

@willcode willcode marked this pull request as draft March 9, 2024 12:57
@willcode
Copy link
Member

willcode commented Mar 9, 2024

Unclear whether this is needed on most systems, and DCO (git signoff) missing, so changing to draft for now.

@GermanAizek
Copy link
Author

@GermanAizek what's your dev setup? Fedora/Ubuntu users certainly won't need this, but what system are you on? Maybe we're making things unnecessarily complicated for some OSs.

Debian and FreeBSD

@marcusmueller
Copy link
Member

Debian? Interesting! So, we're now a bit on a crossroads:

  1. We change nothing, and ccache is opt-in on debian, and opt-out on Fedora, Ubuntu, it seems, through setting -DCMAKE_CXX_COMPILER_LAUNCHER (to empty string to opt out and to ccache to opt in)
  2. We generally adopt the direction of the PR, and fix the PR to at least honor an already defined CMAKE_CXX_COMPILER_LAUNCHER, even and especially when set to empty, thus making ccache opt-out.

In both cases the current form of the PR isn't going to get us where we want!

Copy link
Member

@marcusmueller marcusmueller left a comment

Choose a reason for hiding this comment

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

Sorry, won't do; this needs to respect an already defined CMAKE_{LANGUAGE}_COMPILER_LAUNCHER

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants