Skip to content

Conversation

@GermanAizek
Copy link
Contributor

@GermanAizek GermanAizek commented Dec 27, 2023

@wsor4035
Copy link
Contributor

wsor4035 commented Dec 27, 2023

duplicate/alternate of #13111 ? - if nothing else, #11667 should be linked @Zughy

@wsor4035 wsor4035 added @ Build CMake, build scripts, official builds, compiler and linker errors Maintenance Tasks to keep the codebase and related parts in order, including architectural improvements Performance labels Dec 27, 2023
@GermanAizek
Copy link
Contributor Author

I did not measure CI build speed, I was interested in local build on computer.

@Zughy Zughy added Feature ✨ PRs that add or enhance a feature Roadmap The change matches an item on the current roadmap Roadmap: Needs approval The change is not part of the current roadmap and needs to be approved by coredevs beforehand and removed Roadmap The change matches an item on the current roadmap labels Dec 28, 2023
@lhofhansl
Copy link
Contributor

A complete rebuild takes 2m12s on my 2019 laptop. Do we really need to add complexity to this?
Not opposed, but at least "neutral".

Copy link
Member

@Desour Desour left a comment

Choose a reason for hiding this comment

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

I've already been using ccache for years (I have /usr/lib/ccache/bin/ in my $PATH), and I consider it essential. Waiting minutes at rebuilding each time you checkout a different branch is awful.
=> I support this.


Please add a build option to enable this. One might still want to compile without ccache every now and then, e.g. for benchmarking build speed.

@Desour Desour added Action / change needed Code still needs changes (PR) / more information requested (Issues) Concept approved Approved by a core dev: PRs welcomed! and removed Roadmap: Needs approval The change is not part of the current roadmap and needs to be approved by coredevs beforehand labels Dec 28, 2023
@Desour
Copy link
Member

Desour commented Dec 28, 2023

duplicate/alternate of #13111 ?

Probably not. The cached build artifacts needs to be stored somewhere, you won't get this without modifying the CI stuff.

@Zughy Zughy added Roadmap: supported by core dev PR not adhering to the roadmap, yet some core dev decided to take care of it and removed Concept approved Approved by a core dev: PRs welcomed! labels Jan 21, 2024
@appgurueu
Copy link
Contributor

appgurueu commented Jan 24, 2024

What's the benefit of making changes to our CMakeLists.txt for this? According to https://github.com/ccache/ccache/wiki/CMake, I can just do -D CMAKE_CXX_COMPILER_LAUNCHER=ccache, and this seems to work just fine; I can switch branches and recompile efficiently.

Maybe we should just recommend ccache in the docs?

@GermanAizek
Copy link
Contributor Author

GermanAizek commented Jan 25, 2024

What's the benefit of making changes to our CMakeLists.txt for this? According to https://github.com/ccache/ccache/wiki/CMake, I can just do -D CMAKE_CXX_COMPILER_LAUNCHER=ccache, and this seems to work just fine; I can switch branches and recompile efficiently.

Maybe we should just recommend ccache in the docs?

Do you suggest entering -D parameter every time? I think most people will forget.

@appgurueu
Copy link
Contributor

appgurueu commented Jan 25, 2024

Do you suggest entering -D parameter every time? I think most people will forget.

This is just needed at "configure time", e.g. when invoking cmake. Subsequent make invocations do not require this to be passed. Using ccache should be an explicit choice; it comes at a cost (disk space and, apparently, a minuscule risk of broken builds) and is only really relevant to developers who frequently recompile anyways.

And if people do "forget" to use ccache, it's not much of an issue; their build will just be slightly slower.

(BTW, I removed the linked issue, since that concerned CI, and this PR does not make necessary CI changes.)

@Desour
Copy link
Member

Desour commented Jan 26, 2024

A simple -DENABLE_CCACHE=1 is easier than something like -DCMAKE_CXX_COMPILER_LAUNCHER=ccache -DCMAKE_C_COMPILER_LAUNCHER=ccache (if you also want to enable it for C files). (And this doesn't even include the sloppiness yet.)

Either way, we should have some doc explaining how one can compile minetest with ccache.

@appgurueu
Copy link
Contributor

appgurueu commented Jan 27, 2024

A simple -DENABLE_CCACHE=1 is easier than something like -DCMAKE_CXX_COMPILER_LAUNCHER=ccache -DCMAKE_C_COMPILER_LAUNCHER=ccache (if you also want to enable it for C files). (And this doesn't even include the sloppiness yet.)

(Nitpick: The C files aren't really relevant for performance. We have only one C file (sha256.c). But it's nice for consistency, I suppose.)

What I like about -DCMAKE_CXX_COMPILER_LAUNCHER=ccache is that it should work for pretty much any CMake C++ project. No need to deal with any project-specific flags there.

Anyways, if it's such a trivial shorthand I wouldn't mind; there is virtually no risk in merging that, though I'd be wary of a default of 1.

Either way, we should have some doc explaining how one can compile minetest with ccache.

Agreed.

@Zughy Zughy removed the Action / change needed Code still needs changes (PR) / more information requested (Issues) label Jan 30, 2024
@Desour
Copy link
Member

Desour commented Feb 4, 2024

The option is still missing.

@Desour Desour added the Action / change needed Code still needs changes (PR) / more information requested (Issues) label Feb 4, 2024
@Zughy Zughy added the Adoption needed The pull request needs someone to adopt it. Adoption welcomed! label Mar 2, 2024
@Zughy Zughy closed this Mar 2, 2024
@nigels-com
Copy link

I found my way here after plumbing ccache into the CMakeLists.txt here because I had a release-mode crash that I wanted to debug in CMAKE_BUILD_TYPE=Debug mode.
As a full time C++ developer, ccache for me is in the "don't leave home without it" category of tooling.
I do pretty much expect it to be there by default, always, without needing an opt-in.
When you're switching branches or to and from debug-mode, usually around a 10x speedup.

Having said that, I do understand that for CI/CD builds it is often not practical to manage the cache state, so it's more optional in practice, unless you're optimizing for cost, turn-around time or throughput at scale.

But for interactive development and iteration, highly recommended by default, never personally seen it fail in practice.

Here are some timings on my machine using ccache:

$ /usr/bin/time -v sh -c "cmake . -DCMAKE_BUILD_TYPE=Release -DRUN_IN_PLACE=TRUE -G Ninja && cmake --build ." 2>&1 | grep Elapsed
	Elapsed (wall clock) time (h:mm:ss or m:ss): 0:09.97
$ /usr/bin/time -v sh -c "cmake . -DCMAKE_BUILD_TYPE=Debug -DRUN_IN_PLACE=TRUE -G Ninja && cmake --build ." 2>&1 | grep Elapsed
	Elapsed (wall clock) time (h:mm:ss or m:ss): 0:30.25

If there is anything further needed to "make the case for ccache", happy to engage with that.
And also a little curious about the reluctance, we've all got our own experiences to draw upon.

diff --git a/CMakeLists.txt b/CMakeLists.txt
index 70a027f57..6f19e31c2 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -89,6 +89,12 @@ endif()
 set(ENABLE_UPDATE_CHECKER (NOT ${DEVELOPMENT_BUILD}) CACHE BOOL
        "Whether to enable update checks by default")
 
+find_program(CCACHE_PROGRAM ccache)
+if(CCACHE_PROGRAM)
+       message(STATUS "Using ccache")
+       set_property(GLOBAL PROPERTY RULE_LAUNCH_COMPILE "${CCACHE_PROGRAM}")
+endif()

@sfan5
Copy link
Collaborator

sfan5 commented Jun 18, 2025

I have ccache installed and it's extremely useful for Luanti development but I'm surprised that you would expect build system to pick it up and enable by default. I have never noticed that being the case with other projects.

@nigels-com
Copy link

Perhaps it's the build automation perspective. If you do want to build every push to every branch, it's nice to have the answer (compiler error, link error, test failure) as soon as possible. If I've broken Android, or ARM, that's what the CI/CD robot is there to catch while it's still reasonably fresh in my mind. I guess I'm too lazy to opt in to something that seems like it ought to be the default. (On Linux, at least)

@appgurueu
Copy link
Contributor

Using ccache in CI is a different PR: #13111

There are tradeoffs involved here. Such a cache takes space (not everyone has plenty of that), and it can fail (and ccache or not, there have definitely been instances where a full rebuild fixed a problem). I would not expect such a cache to be enabled by default just because I have the relevant program installed. I would want to (and currently do) explicitly opt in.

I'm lazy too but passing a couple command-line arguments when configuring is very much acceptable.

@nigels-com
Copy link

Some things I'd mention in favour of ccache by default, for Linux at least.

  • It's reliable. I don't deny that it seems a bit magical. It's a little bit of a leap of faith. Expect up to a 10x speedup long term.
  • The default cache size is 5GB. A 1GB cache seems more than sufficient for luanti. $ ccache -M 1GB
  • ccache has an opt-out in the form of CCACHE_DISABLE=true environment variable.
  • Using cmake does already imply a degree of auto-detection. It's usually not necessary to tell it which compiler to use, for example. Having an opinion and good defaults goes with that territory.
  • Setting this up in CMakeLists.txt implies that luanti devs trust, recommend and default to working in this manner.
  • Take a glance at the manual and be assured that ccache is a mature, serious and well documented tool.
  • Probably better known to Linux and Mac developers than Windows ones.

So as a demonstration for luanti on a 24-core AMD Ryzen 9 7900 with a pre-warmed cache

$ ccache -s
...
Local storage:
  Cache size (GB):  0.3 /  1.0 (34.38%)

Building with ccache:

$ /usr/bin/time -v sh -c "cmake . -DCMAKE_BUILD_TYPE=Debug -DRUN_IN_PLACE=TRUE -G Ninja && CCACHE_NODISABLE=true cmake --build ." 2>&1 | grep Elapsed
	Elapsed (wall clock) time (h:mm:ss or m:ss): 0:30.43
$ /usr/bin/time -v sh -c "cmake . -DCMAKE_BUILD_TYPE=Release -DRUN_IN_PLACE=TRUE -G Ninja && CCACHE_NODISABLE=true cmake --build ." 2>&1 | grep Elapsed
	Elapsed (wall clock) time (h:mm:ss or m:ss): 0:10.09

And without:

$ /usr/bin/time -v sh -c "cmake . -DCMAKE_BUILD_TYPE=Debug -DRUN_IN_PLACE=TRUE -G Ninja && CCACHE_DISABLE=true cmake --build ." 2>&1 | grep Elapsed
	Elapsed (wall clock) time (h:mm:ss or m:ss): 0:55.14
$ /usr/bin/time -v sh -c "cmake . -DCMAKE_BUILD_TYPE=Release -DRUN_IN_PLACE=TRUE -G Ninja && CCACHE_DISABLE=true cmake --build ." 2>&1 | grep Elapsed
	Elapsed (wall clock) time (h:mm:ss or m:ss): 0:33.82

This is a relatively strong CPU on a new system.
The fewer the cores and lesser the Mhz, the bigger the speedup.

@Desour
Copy link
Member

Desour commented Jun 19, 2025

I don't see the benefit of using ccache in the cmakelists by default:

  • If the user doesn't know about ccache, they won't have it installed, and won't benefit.
  • If the user wants to compile everything with ccache by default, they will already have /usr/lib/ccache/bin/ in their PATH. (I do this.)
  • Otherwise, the user doesn't want this default.

Someone should just add mention of ccache here: https://docs.luanti.org/for-engine-devs/compiling/improving-build-times/

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

Labels

Action / change needed Code still needs changes (PR) / more information requested (Issues) Adoption needed The pull request needs someone to adopt it. Adoption welcomed! @ Build CMake, build scripts, official builds, compiler and linker errors Feature ✨ PRs that add or enhance a feature Maintenance Tasks to keep the codebase and related parts in order, including architectural improvements Performance Roadmap: supported by core dev PR not adhering to the roadmap, yet some core dev decided to take care of it

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants