Default CMAKE_BUILD_TYPE to Release?

[wxmerkt]

A common gotcha for new users is not having their CMAKE_BUILD_TYPE set and experiencing low performance. Does it make sense to (a) set the available default types and (b) default to Release if not specified by the user?

I am thinking of adding the following as e.g. in Crocoddyl to base.cmake (or a similar file):

# Set a default build type to 'Release' if none was specified
IF(NOT CMAKE_BUILD_TYPE AND NOT CMAKE_CONFIGURATION_TYPES)
  MESSAGE(STATUS "Setting build type to 'Release' as none was specified.")
  SET(CMAKE_BUILD_TYPE Release CACHE STRING "Choose the type of build." FORCE)
  # Set the possible values of build type for cmake-gui
  SET_PROPERTY(CACHE CMAKE_BUILD_TYPE PROPERTY STRINGS "Debug" "Release" "MinSizeRel" "RelWithDebInfo")
ENDIF()

What are your thoughts @jcarpent @nim65s @gergondet ?

[nim65s]

This would break the CMAKE_BUILD_TYPE environment variable. This would produce undesired behavior for packagers, especially on Arch. This would trick our users into believing that by default, CMake produce optimized code, so when they will use a package we don't own and which follows CMake defaults, they won't understand why everything is so slow.

So I think it would be better to add a little example in the README.md like

cmake -B build -S . -DCMAKE_BUILD_TYPE=Release
cmake --build build

[gergondet]

I think there is a small misunderstanding by @nim65s because I think what @wxmerkt proposes does not override the CMAKE_BUILD_TYPE:

• if it's set through the environment variable
• if it's set to None: I think Arch does this explicitly because the "true" default of CMAKE_BUILD_TYPE is empty, at the very least, I'm 100% sure this what Debian packaging tools do

Regarding the last point, I get what you mean but we can still add the -DCMAKE_BUILD_TYPE=... in instructions and work-around the default of CMake.

However, while I like the idea, I would argue RelWithDebInfo is a much better default than Release. The difference in performance between the two is often minimal and having access to debug symbols is well worth it.

[olivier-stasse]

If I agree with most of @gergondet arguments, there is currently a problem with the ROS2 buildfarm due to memory exhaustion while linking for pinocchio. It is using flags very similar to RelWithDebInfo: -g -O2. The memory exhaustion is probably due to the symbols embedded with the -g flag. As they dgbsym packages provided by the buildfarm which acts as an alternative, i do not see very much the interest of RelWithDebInfo.

So I would be in favor of @wxmerkt proposal.

[gergondet]

In my understanding what @wxmerkt is proposing is to set a CMAKE_BUILD_TYPE if none is provided which is a common mistake for people unaware of CMake's default behavior (typically beginners). More experienced users and packaging tools do not do that and set an explicit CMAKE_BUILD_TYPE that matches their needs. For those beginners having debug symbols enabled by default seems like a good idea.

It is using flags very similar to RelWithDebInfo: -g -O2.

Probably it's the default CMAKE_BUILD_TYPE=None set by Debian packaging tools? It's basically RelWithDebInfo but without -DNDEBUG. The mechanism that @wxmerkt is proposing would not override that.

We can also imagine a simple mechanism in the modules to override this default choice but I still believe RelWithDebInfo is a better default.

[jcarpent]

i do not see very much the interest of RelWithDebInfo.

I would say for the O2 optimization, which is less aggressive than O3.

[olivier-stasse]

Probably it's the default CMAKE_BUILD_TYPE=None set by Debian packaging tools? It's basically RelWithDebInfo but without -DNDEBUG. The mechanism that @wxmerkt is proposing would not override that.

If this is the case then this will not fix the matter that we have in ROS...

[wxmerkt]

Just checked and indeed "None" would not trigger the above mechanism. Would overriding it with a sensible default be fine if it is None or does this have drawbacks (and if so which?)?

[nim65s]

I think what @wxmerkt proposes does not override the CMAKE_BUILD_TYPE:

* if it's set through the environment variable

I think it does. Here is a little test:

export CMAKE_BUILD_TYPE=RelWithDebInfo

for project in ndcurves crocoddyl
do
    rm -rf ./$project
    git clone --recursive https://github.com/loco-3d/$project
    cmake -B ./$project/build -S ./$project
done

grep CMAKE_BUILD_TYPE: */build/CMakeCache.txt

crocoddyl has the snipped we are talking about, and ndcurves doesn't. Here is the result:

crocoddyl/build/CMakeCache.txt:CMAKE_BUILD_TYPE:STRING=Release
ndcurves/build/CMakeCache.txt:CMAKE_BUILD_TYPE:STRING=RelWithDebInfo

[nim65s]

I'm not saying I am against it because this would break something for me (because this would not: I'll be fine without a CMAKE_BUILD_TYPE environment variable), but I think we need to take a lot of care if we try to provide different default settings than everybody else.

[wxmerkt]

So my new proposal is as follows:

# Set a default build type to 'Release' if none was specified
IF((NOT CMAKE_BUILD_TYPE OR CMAKE_BUILD_TYPE STREQUAL "None") AND NOT CMAKE_CONFIGURATION_TYPES)
  MESSAGE(STATUS "Setting build type to 'Release' as none was specified.")
  SET(CMAKE_BUILD_TYPE Release CACHE STRING "Choose the type of build." FORCE)
  # Set the possible values of build type for cmake-gui
  SET_PROPERTY(CACHE CMAKE_BUILD_TYPE PROPERTY STRINGS "Debug" "Release" "MinSizeRel" "RelWithDebInfo")
ELSE()
  MESSAGE(STATUS "Using build type: ${CMAKE_BUILD_TYPE} / ${CMAKE_CONFIGURATION_TYPES}")
ENDIF()

[gergondet]

Just checked and indeed "None" would not trigger the above mechanism. Would overriding it with a sensible default be fine if it is None or does this have drawbacks (and if so which?)?

This is part of the CMake package guidelines that were linked above in the conversation and I think you can find similar things in other packaging guides. The rationale for distribution is to make sure CMake based packages are built with similar flags and keeping the assert in allow them to better trace bugs.

See https://lists.debian.org/debian-med/2018/04/msg00121.html for a conversation on this topic and the "sanctioned" way to disable asserts in Debian packaging.

In a nutshell, I don't think we should automatically override None unless it really makes sense for a particular project.

So my new proposal is as follows:

According to @nim65s this should also check for the ENV{CMAKE_BUILD_TYPE} existence.

[wxmerkt]

What would be a good solution then?

As far as I understand, there is no consensus on overriding if set to None in case someone is packaging for Debian. As thus, would it alternatively be fine to set -O2 -g to CMAKE_CXX_FLAGS (although I would prefer setting CMAKE_BUILD_TYPE over messing with CMAKE_CXX_FLAGS)? It would be greatly beneficial for performance to have some sensible default being auto-set - open to suggestions what those would entail.

# Set a default build type to 'Release' if none was specified
IF((NOT CMAKE_BUILD_TYPE OR CMAKE_BUILD_TYPE STREQUAL "None") AND NOT CMAKE_CONFIGURATION_TYPES AND NOT ENV{CMAKE_BUILD_TYPE})
  MESSAGE(STATUS "Setting CMAKE_CXX_FLAGS to include O2 build type to 'Release' as none was specified.")
  SET(CMAKE_CXX_FLAGS "-O2 -g ${CMAKE_CXX_FLAGS}")
ELSE()
  MESSAGE(STATUS "Using build type: ${CMAKE_BUILD_TYPE} / ${CMAKE_CONFIGURATION_TYPES}")
ENDIF()

[gergondet]

would it alternatively be fine to set -O2 -g to CMAKE_CXX_FLAGS

This is what CMAKE_BUILD_TYPE=None does.

What would be a good solution then?

If the goal is to set a reasonable default for CMAKE_BUILD_TYPE when it's not provided then I'm personally all for it. We can settle on a default that can be overridden (e.g. via PROJECT_DEFAULT_CMAKE_BUILD_TYPE) and ship that.

If the goal is to work-around a memory limit in the ROS2 build farm by overwriting the CMAKE_BUILD_TYPE that is set by the packaging tools then I would suggest to find another approach as there must be some way to pass specific options to CMake when building the guilty packages in that environment (e.g. disable debug information for those specific packages or otherwise limit the size of the debug information via debug options)

[wxmerkt]

Thank you for the insightful comment. I was aiming to solve two problems at the same time. For the packaging, I am going with another approach now (which unfortunately requires additional time effort).

I still believe it makes sense as stated in the original post to ship a mechanism to automatically configure a CMAKE_BUILD_TYPE if none is configured -- for a source builds of package users, I see the mistake of not configuring it often enough and then having issues with performance.

[gergondet]

I still believe it makes sense as stated in the original post to ship a mechanism to automatically configure a CMAKE_BUILD_TYPE if none is configured -- for a source builds of package users, I see the mistake of not configuring it often enough and then having issues with performance.

To re-iterate, I 100% agree with this :)

[jcarpent]

FIne on my side too.

[jcarpent]

I've provided a routine to set the default CMAKE_BUILD_TYPE via #593. We should now decide if we set the behavior by default with the Release config. @jmirabel @nim65s @olivier-stasse @gergondet @wxmerkt What is your opinion?

[olivier-stasse]

I am too for a default CMAKE_BUILD_TYPE set to RELEASE.

[jmirabel]

I have no strong opinion.

I would say people that are not aware of CMake build type and compilation flags are also not very likely to use debug symbols. So they shouldn't be taken into account for choosing between Release or RelWithDebInfo.

I agree it is convenient of have some kind of optimization by default. CMake doc says

The default value is often an empty string, but this is usually not desirable and one of the other standard build types is usually more appropriate.

I have seen using Release by default in many other packages using CMake.

[wxmerkt]

I usually tend to default to RelWithDebInfo since it gives us the best of both worlds, and being able to tell people "can you run gdb and backtrace" for debugging is a significant benefit/timesaver compared with having to explain "change CMAKE_BUILD_TYPE, clean, rebuild, run again and then ...". That said, projects such as EigenPy and Pinocchio require a lot more RAM and time to build with debug symbols so that might cause other user issues - hence, happy to go with whichever Release-type flag you consider most suitable :-)

[gergondet]

Given that the added macro takes the default build type as an option I don't think we have to pick one or are you intending to automatically call this macro in init project? If so, imho, it should remain overridable (e.g. via PROJECT_DEFAULT_CMAKE_BUILD_TYPE)

I side with @wxmerkt I think RelWithDebInfo is generally a better choice except when debug info production is problematic on lower-end setups.

[nim65s]

I'd agree for putting RelWithDebInfo by default everywhere, let individual projects change this default (eg. to Release for pinocchio), and let indivdual users change it via standard CMake ways, as already implemented in #592 & #593.

[nim65s]

On a second thought, I think we might need to add:

1. a message(STATUS …) when this is activated
2. a flag deactivating this behaviour (eg. for Arch packaging which want an empty CMAKE_BUILD_TYPE)

But this is not blocking for anything else. I'll open a PR for this.

[gergondet]

a flag deactivating this behaviour (eg. for Arch packaging which want an empty CMAKE_BUILD_TYPE)

Wouldn't it work if you pass -DCMAKE_BUILD_TYPE="" in that particular case?

[nim65s]

You're right, but with -DCMAKE_BUILD_TYPE=None I think.

[gergondet]

I don't know about Arch but on Debian/Ubuntu at least, None and empty are different settings (None is basically RelWithDebInfo but without -DNDEBUG)

[nim65s]

I didn't went to CMake docs to double check what is happening, but Arch' really want this -DCMAKE_BUILD_TYPE=None: https://wiki.archlinux.org/title/CMake_package_guidelines#Fixing_the_automatic_optimization_flag_override

[gergondet]

My bad, in fact None is basically a trick to work-around the automatic setting of CMAKE_BUILD_TYPE when it is empty. It seems the optimization flags (-O2 -g) are then added in another way by the packaging tools (in Debian/Ubuntu and so I guess in Arch?)

[nim65s]

Yes, in Arch default /etc/makepkg.conf:

# grep -A2 O2 /etc/makepkg.conf  
CFLAGS="-march=x86-64 -mtune=generic -O2 -pipe -fno-plt -fexceptions \
        -Wp,-D_FORTIFY_SOURCE=2 -Wformat -Werror=format-security \
        -fstack-clash-protection -fcf-protection"