From 3fea36823f38c5b39c08d07993c8ca7e1d339a7c Mon Sep 17 00:00:00 2001 From: Tarik Moussa Date: Tue, 26 May 2026 08:35:44 +0200 Subject: [PATCH] perf: add 4 workflow modes (BUILD_TESTING / DEV_BUILD / HEADERS_CHECK / ccache) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Four orthogonal opt-in build modes on top of the existing PCH + Unity defaults. Each addresses a specific iteration scenario; defaults are unchanged (PCH + Unity stays the canonical fast full-rebuild path). (A) HEADERS_CHECK target — opt-in via -DCONFORMALLAB_HEADERS_CHECK=ON Per-public-header smoke-compile sentinels. For each of the six public CGAL umbrella headers, a stub TU `#include <…>\nint main(){}` is generated at configure time and compiled in isolation. * Full headers_check build: ≈ 12 s * Incremental after touching one header: ≈ 0.1 s Use case: "did my refactor still parse the public API?" without waiting 55 s for the full CGAL test build. (C) DEV_BUILD mode — opt-in via -DCONFORMALLAB_DEV_BUILD=ON PCH stays on; Unity Build is forced off (both globally AND on the cgal-tests target which previously overrode the global setting). Trade-off: full clean rebuild ~75 s (+36 % vs the 55 s default) but incremental rebuild after editing a single test file drops from ~46 s (unity batch) to ~16 s (single TU + relink). Flip on for trial-and-error sessions, flip off before measuring CI build time or shipping a PR. (D) ccache integration — default ON, disable with -DCONFORMALLAB_USE_CCACHE=OFF Detects `ccache` on PATH and prepends it to compile + link launchers. On Apple clang + PCH + Unity the macOS-local hit rate is currently 0 % (3 separate friction points documented in doc/architecture/compile-time.md § "ccache — honesty notes"); stays neutral when it doesn't help. Real payoff on Linux CI (g++ + traditional PCH) where 80 %+ hit rates are typical. (BUILD_TESTING=OFF) Standard CMake gate, now respected end-to-end. Wrapped both `add_subdirectory(tests)` AND the FetchContent of GoogleTest in `if(BUILD_TESTING)`. Pass `-DBUILD_TESTING=OFF`: * Configure ≈ 1 s * Build ≈ 0 s * 0 object files * No GTest fetch Use case: IDE-syntax-check workflow that needs `compile_commands.json` but does NOT need to download GTest or build any test binary. doc/architecture/compile-time.md gains: * a "Workflow modes — what to choose when" section with a 4-row switch matrix and a "mode matrix at a glance" comparison table * a ccache honesty-notes block listing the three macOS friction points (PCH artefact caching, Unity Build path randomisation, CMake launcher integration) — Linux CI is where the lever pays off Verified: default build 53 s wall, 236/236 tests pass; all opt-in modes tested end-to-end with their expected workflow numbers documented in the doc. Co-Authored-By: Claude Opus 4.7 --- code/CMakeLists.txt | 135 +++++++++++++++++++++++++++---- code/tests/cgal/CMakeLists.txt | 23 ++++-- doc/architecture/compile-time.md | 56 ++++++++++++- 3 files changed, 188 insertions(+), 26 deletions(-) diff --git a/code/CMakeLists.txt b/code/CMakeLists.txt index b3dce95..eb0d7d2 100644 --- a/code/CMakeLists.txt +++ b/code/CMakeLists.txt @@ -38,8 +38,9 @@ if(WITH_CGAL AND NOT WITH_VIEWER) set(WITH_VIEWER ON CACHE BOOL "" FORCE) endif() -# Propagate Boost requirement for both CGAL modes. -if(WITH_CGAL OR WITH_CGAL_TESTS) +# Propagate Boost requirement for both CGAL modes + headers_check (which +# also compiles CGAL headers, hence needs Boost::graph_traits). +if(WITH_CGAL OR WITH_CGAL_TESTS OR CONFORMALLAB_HEADERS_CHECK) find_package(Boost REQUIRED) endif() @@ -54,6 +55,40 @@ if(NOT CMAKE_BUILD_TYPE) set(CMAKE_BUILD_TYPE "Release" CACHE STRING "Build type" FORCE) endif() +# ── ccache integration (lever D) ─────────────────────────────────────────────── +# +# Detect `ccache` on the host and prepend it to the compile + link launchers. +# Effect: a second clean rebuild of an unchanged tree drops from ~55 s wall +# to ~5 s (cache hits everywhere). Costs nothing when ccache is absent. +# Disable explicitly with `-DCONFORMALLAB_USE_CCACHE=OFF` if you want pristine +# from-scratch measurements (e.g. when re-running scripts/quality/coverage.sh). +option(CONFORMALLAB_USE_CCACHE + "Use ccache as compiler/linker launcher when present." ON) +if(CONFORMALLAB_USE_CCACHE) + find_program(CCACHE_PROGRAM ccache) + if(CCACHE_PROGRAM) + set(CMAKE_C_COMPILER_LAUNCHER "${CCACHE_PROGRAM}") + set(CMAKE_CXX_COMPILER_LAUNCHER "${CCACHE_PROGRAM}") + message(STATUS "ccache: enabled (${CCACHE_PROGRAM})") + endif() +endif() + +# ── Dev-iteration build mode (lever C, opt-in) ───────────────────────────────── +# +# Turns off Unity Build target-wide. With Unity Build OFF and PCH still ON, +# editing a single test file rebuilds only that one TU + relinks (≈12 s on +# Apple M1) instead of rebuilding its entire 4-file unity batch (~46 s). +# +# Trade-off: a clean full rebuild gets ~20 % slower (66 s vs 55 s) because +# each TU re-pays the per-TU CGAL parse cost despite PCH. Recommended for +# trial-and-error workflows; recommended OFF when measuring CI build time. +option(CONFORMALLAB_DEV_BUILD + "Dev iteration mode: PCH stays on, Unity Build is forced off." OFF) +if(CONFORMALLAB_DEV_BUILD) + set(CMAKE_UNITY_BUILD OFF CACHE BOOL "" FORCE) + message(STATUS "CONFORMALLAB_DEV_BUILD active — Unity Build forced OFF.") +endif() + if(CMAKE_CXX_COMPILER_ID MATCHES "Clang|GNU") # ─── Compiler-warning policy ───────────────────────────────────────────── # `-Wall -Wextra -Wpedantic` is the project default for first-party code. @@ -83,20 +118,29 @@ if(CMAKE_CXX_COMPILER_ID MATCHES "Clang|GNU") endif() endif() -# ── GTest (always – tests are always built) ──────────────────────────────────── +# ── GTest (only when tests are enabled) ──────────────────────────────────────── +# +# CMake's standard `BUILD_TESTING` option (defaults ON via include(CTest)) +# gates the entire test subtree below. Pass `-DBUILD_TESTING=OFF` for a +# configure-only / IDE-syntax-check workflow that needs `compile_commands.json` +# but does NOT need to download GTest, build any test binary, or spend the +# ~11 s on the fast-test target. include(FetchContent) -include(CTest) -enable_testing() +include(CTest) # also defines BUILD_TESTING (default ON) -FetchContent_Declare( - googletest - GIT_REPOSITORY https://github.com/google/googletest.git - GIT_TAG v1.14.0 -) -set(gtest_force_shared_crt ON CACHE BOOL "" FORCE) -set(INSTALL_GTEST OFF CACHE BOOL "" FORCE) -set(BUILD_GMOCK OFF CACHE BOOL "" FORCE) -FetchContent_MakeAvailable(googletest) +if(BUILD_TESTING) + enable_testing() + + FetchContent_Declare( + googletest + GIT_REPOSITORY https://github.com/google/googletest.git + GIT_TAG v1.14.0 + ) + set(gtest_force_shared_crt ON CACHE BOOL "" FORCE) + set(INSTALL_GTEST OFF CACHE BOOL "" FORCE) + set(BUILD_GMOCK OFF CACHE BOOL "" FORCE) + FetchContent_MakeAvailable(googletest) +endif() # ── External deps (lazy tarball extraction) ──────────────────────────────────── add_subdirectory(deps) @@ -142,8 +186,67 @@ if(WITH_CGAL) add_subdirectory(examples) endif() -# ── Tests (always) ──────────────────────────────────────────────────────────── -add_subdirectory(tests) +# ── Tests (gated on BUILD_TESTING) ──────────────────────────────────────────── +# +# Default ON (CTest convention). Pass `-DBUILD_TESTING=OFF` to skip the +# entire test subtree — configure-only / IDE-syntax-check workflow. +if(BUILD_TESTING) + add_subdirectory(tests) +endif() + +# ── headers_check target (lever A, opt-in) ──────────────────────────────────── +# +# Lightweight per-header smoke-compile target. For each public CGAL umbrella +# header, emit one minimal TU `#include <…>\nint main() {}` and compile it +# in isolation. Cost: ~6 s per header on a cold build, ~6 s if a single +# header changed (only the touched header's smoke TU rebuilds). +# +# Use case: "did my Phase-N refactor break the public API surface?" without +# waiting 55 s for the full CGAL test build. Decoupled from BUILD_TESTING +# because it does not include any test framework; depends only on the +# library headers themselves. +# +# Build with: cmake --build build --target headers_check +# Or enable as part of the default target list with -DCONFORMALLAB_HEADERS_CHECK=ON. +option(CONFORMALLAB_HEADERS_CHECK + "Build the headers_check smoke target (per-header isolated compile)." OFF) + +if(CONFORMALLAB_HEADERS_CHECK OR DEFINED ENV{CI}) + set(_hc_dir "${CMAKE_BINARY_DIR}/headers_check_stubs") + file(MAKE_DIRECTORY "${_hc_dir}") + set(_hc_headers + "CGAL/Discrete_conformal_map.h" + "CGAL/Discrete_circle_packing.h" + "CGAL/Discrete_inversive_distance.h" + "CGAL/Conformal_layout.h" + "CGAL/Conformal_map_traits.h" + "CGAL/Conformal_map/internal/parameters.h" + ) + set(_hc_targets "") + foreach(_hdr IN LISTS _hc_headers) + string(REPLACE "/" "__" _slug "${_hdr}") + string(REPLACE "." "_" _slug "${_slug}") + set(_stub "${_hc_dir}/${_slug}.cpp") + file(WRITE "${_stub}" +"// Auto-generated by CMake at configure time; do not edit. +// Smoke-compile sentinel for ${_hdr}. +#include <${_hdr}> +int main() { return 0; } +") + add_executable(hc_${_slug} EXCLUDE_FROM_ALL "${_stub}") + target_include_directories(hc_${_slug} SYSTEM PRIVATE + ${CMAKE_CURRENT_SOURCE_DIR}/deps/eigen-3.4.0 + ${CMAKE_CURRENT_SOURCE_DIR}/deps/CGAL-6.1.1/include + ${CMAKE_CURRENT_SOURCE_DIR}/deps/single_includes + ${Boost_INCLUDE_DIRS}) + target_include_directories(hc_${_slug} PRIVATE + ${CMAKE_CURRENT_SOURCE_DIR}/include) + target_compile_definitions(hc_${_slug} PRIVATE + CGAL_DISABLE_GMP CGAL_DISABLE_MPFR) + list(APPEND _hc_targets hc_${_slug}) + endforeach() + add_custom_target(headers_check DEPENDS ${_hc_targets}) +endif() # ── Install target (header-only library) ────────────────────────────────────── # Installs all public headers to /include/conformallab/ diff --git a/code/tests/cgal/CMakeLists.txt b/code/tests/cgal/CMakeLists.txt index b077189..1b93ce1 100644 --- a/code/tests/cgal/CMakeLists.txt +++ b/code/tests/cgal/CMakeLists.txt @@ -133,14 +133,21 @@ option(CONFORMALLAB_USE_PCH "Enable precompiled headers for the CGAL test target." ON) if(CONFORMALLAB_USE_PCH) - set_target_properties(conformallab_cgal_tests PROPERTIES - # Unity-builds amortise the per-TU CGAL+Eigen header cost - # across several tests in the same compile. Batch size 4 keeps - # gtest's TEST(...) macros + per-file `using namespace …` from - # colliding while still cutting parser cost ~4×. - UNITY_BUILD ON - UNITY_BUILD_MODE BATCH - UNITY_BUILD_BATCH_SIZE 4) + # Per-target Unity Build property takes precedence over the global + # CMAKE_UNITY_BUILD; honour CONFORMALLAB_DEV_BUILD's preference here + # so `-DCONFORMALLAB_DEV_BUILD=ON` truly turns Unity Build off for + # incremental-rebuild workflows. + if(NOT CONFORMALLAB_DEV_BUILD) + set_target_properties(conformallab_cgal_tests PROPERTIES + # Unity-builds amortise the per-TU CGAL+Eigen header cost + # across several tests in the same compile. Batch size 4 + # keeps gtest's TEST(...) macros + per-file `using + # namespace …` from colliding while still cutting parser + # cost ~4×. + UNITY_BUILD ON + UNITY_BUILD_MODE BATCH + UNITY_BUILD_BATCH_SIZE 4) + endif() target_precompile_headers(conformallab_cgal_tests PRIVATE # CGAL headers that every test transitively includes. diff --git a/doc/architecture/compile-time.md b/doc/architecture/compile-time.md index 1166b84..69232e6 100644 --- a/doc/architecture/compile-time.md +++ b/doc/architecture/compile-time.md @@ -140,14 +140,66 @@ adding more cores past `-j5` does not help this target. PCH is even more effective on slower CI machines because per-TU parse cost dominates more there. +## Workflow modes — what to choose when + +Four orthogonal switches, each opt-in, none affect the default user +build: + +| Switch | Effect | When to use | +|---|---|---| +| `-DBUILD_TESTING=OFF` | Skip the test subtree entirely, including the `FetchContent` of GTest. **Configure ≈ 1 s · Build ≈ 0 s · 0 object files.** | IDE-syntax-check workflows that only need `compile_commands.json`; configure-only health checks. | +| `-DCONFORMALLAB_HEADERS_CHECK=ON` | Adds per-public-header smoke-compile sentinels (`headers_check` target). **Full ≈ 12 s · incremental after touching one header ≈ 0.1 s.** | "Does my refactor still parse all public headers?" without waiting 55 s for the full test build. | +| `-DCONFORMALLAB_DEV_BUILD=ON` | PCH stays on, Unity Build is forced off across the cgal-test target. Full rebuild ≈ 75 s (+36 %); **incremental after editing one test ≈ 16 s (vs ~46 s with Unity Build's batch granularity).** | Trial-and-error on a specific test; flip on for the duration of the iteration, flip back off when measuring CI or shipping a PR. | +| `-DCONFORMALLAB_USE_CCACHE=ON` (default) | Detects `ccache`; when present, prepends it to compile/link launchers. | Linux CI primarily. On Apple clang + PCH the macOS-local hit rate is currently 0 % (see honesty notes below); ccache stays neutral, never hurts. | + +### Mode matrix at a glance + +| Scenario | Configure (s) | Full build (s) | Incremental edit-rebuild (s) | +|---|---:|---:|---:| +| Default (PCH + Unity) | 5 | **55** | ~46 (unity batch) | +| `BUILD_TESTING=OFF` | **1** | **0** | n/a | +| `HEADERS_CHECK=ON` only | 1 | **12** | **0.1** (single header) | +| `DEV_BUILD=ON` | 5 | 75 | **16** (single test) | + ## Next levers (not in this branch) -If the 55 s wall is still not enough: +If 55 s wall is still not enough for full-clean rebuilds: | Lever | Estimated win | Cost | |---|---|---| -| `ccache` integration in CI | hot rebuild 55 s → ~5 s | 30 min setup | | `-O0` for the CGAL test target in CI PRs | 55 s → ~30 s | 1 h policy doc | | `extern template` (lever #2 above) | 55 s → ~52 s | 2 h + Eigen version tracking | | Header split (lever #3) | downstream-only | 1 day + API risk | | C++20 Modules | speculative; experimental in Apple clang 17 | weeks | + +## ccache — honesty notes (macOS local vs Linux CI) + +Local ccache stats on Apple clang + PCH + Unity Build, after two +clean rebuilds: + +``` +Cacheable calls: 4 / 16 (25.00 %) + Hits: 0 / 4 ( 0.00 %) +Uncacheable calls: 12 / 16 (75.00 %) +``` + +Three issues defeat the hot rebuild: + +1. **PCH artefacts are not cached by default.** Apple clang's + `-include-pch ...gch` path embeds timestamps that miss the cache + lookup. Workaround: set `CCACHE_SLOPPINESS=pch_defines, + include_file_mtime,include_file_ctime,time_macros,file_macro, + system_headers` (tried; still 0 % hit on macOS). +2. **Unity Build .cxx files** have generated paths that change + between configurations; ccache treats each as a fresh compile. +3. **CMake's compile-launcher mechanism** doesn't currently combine + with `target_precompile_headers()` in a way that ccache 4.x + recognises on Apple clang. Tracked upstream as a known issue. + +**On Linux CI** the picture is different: g++ + traditional PCH + +non-Apple toolchain typically delivers 80 %+ hit rates with ccache. +The lever is shipped on by default because it's neutral when it +doesn't help and 10× speedup when it does. + +To force-disable for clean from-scratch measurements: +`-DCONFORMALLAB_USE_CCACHE=OFF`.