make Release builds RelWithDebug
Nobody has claimed this yet.
- Dominant language
- CMake
- Stars
- 116
- Forks
- 18
- PR merge metrics
- No merged PRs in 30d
Description
@GretaCB - Now that benchmarks are landing in #48, the problem arises of how to best profile those benchmarks. As we did last week, we will be profiling the benchmarks to get calltrees of what is taking time in order to understand where we might optimize. The challenge is that those calltrees are often truncated due to inlining. But we should be careful not to make decisions on debug builds, which change performance of the binaries.
In an ideal world we could have release binaries (that run as fast as possible) but that also have beautiful calltrees as if the binaries are not optimized by inlining.
While the world is not ideal, we can approximate this by using `RelWithDebug` which will add `-g`. Then we can also add `-fno-inline-functions`. This latter flag with disable inlining only (and all other optimizations will still be applied). This is dangerous for production binaries we might deploy (since it could slow the code down), but safe for benchmarks (as long as the slower code is not too unrepresentative of reality). See https://github.com/mapbox/cpp/blob/master/glossary.md#problem-of-debugging-release-crashes for more details.
To recap, I think we should test doing:
```diff
diff --git a/CMakeLists.txt b/CMakeLists.txt
index 17405d7..f64844b 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -13,7 +13,8 @@ if (CMAKE_BUILD_TYPE STREQUAL "Debug")
set(OPTIMIZATION_FLAGS "-O0 -DDEBUG")
message("-- Configuring debug build")
else()
- set(OPTIMIZATION_FLAGS "-O3 -DNDEBUG")
+ # https://github.com/mapbox/cpp/blob/master/glossary.md#profiling-build
+ set(OPTIMIZATION_FLAGS "-O3 -DNDEBUG -fno-inline-functions")
message("-- Configuring release build")
endif()
diff --git a/Makefile b/Makefile
index ce7a832..98b8867 100644
--- a/Makefile
+++ b/Makefile
@@ -5,7 +5,8 @@ export WERROR ?= true
default: release
release:
- mkdir -p build && cd build && cmake ../ -DCMAKE_BUILD_TYPE=Release -DWERROR=$(WERROR) && VERBOSE=1 cmake --build .
+ # https://github.com/mapbox/cpp/blob/master/glossary.md#debuggable-release-build
+ mkdir -p build && cd build && cmake ../ -DCMAKE_BUILD_TYPE=RelWithDebInfo -DWERROR=$(WERROR) && VERBOSE=1 cmake --build .
debug:
mkdir -p build && cd build && cmake ../ -DCMAKE_BUILD_TYPE=Debug -DWERROR=$(WERROR) && VERBOSE=1 cmake --build .
```
If this does not slow down the benchmark results more than 5% and also results in better calltrees (no major missing data for in Activity Monitor like we saw in release mode), then I would feel comfortable with this being committed.
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start with CMakeLists.txt and Makefile, then inspect the release and debug build targets and the benchmark setup from issue #48. Build the proposed release configuration, profile the benchmarks, and compare the results against the stated 5% slowdown limit and the quality of the resulting calltrees.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- cmake, cpp
- Domain
- build-system, performance
- Issue type
- Feature
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Stale
- Clarity
- Clearly specified
- Newbie friendliness
- 45/100