morningman commented on code in PR #66712: URL: https://github.com/apache/doris/pull/66712#discussion_r3774461293
########## be/CMakeLists.txt: ########## @@ -80,6 +80,10 @@ add_definitions(-DGLOG_CUSTOM_PREFIX_SUPPORT) option(GLIBC_COMPATIBILITY "Enable compatibility with older glibc libraries." ON) option(USE_LIBCPP "Use libc++" OFF) option(USE_JEMALLOC "Use jemalloc" ON) +# Merge groups of .cpp files into jumbo translation units for much faster full +# builds. Turn OFF for precise per-file diagnostics, per-file tooling +# (clang-tidy/coverage), or the finest-grained incremental rebuilds. +option(ENABLE_UNITY_BUILD "Enable CMake unity builds for BE targets" ON) Review Comment: Fixed in 90d33da6061, taking the second of your two options: `load_tus()` now counts unity blocks and exits with the rebuild instruction (`ENABLE_UNITY_BUILD=OFF ./build.sh --compile-bench`) instead of returning a truncated TU set. Expanding the block is not a safe alternative. A unity block carries the *union* of its members' closures, so attributing it back to each member would make every source look like it reaches every header any sibling pulls in — inflating both the affected-TU counts and the seeding advice that `edge`/`audit` emit. Silent under-coverage would be traded for silent over-estimation, and this tool exists to be precise about exactly that. Fixture added: `build-support/tests/test-compile-bench-unity.sh` feeds `load_tus()` a canned `ninja -t deps` database in both shapes and asserts the standalone block is still kept and the unity block is refused with the instruction. ########## be/CMakeLists.txt: ########## @@ -80,6 +80,10 @@ add_definitions(-DGLOG_CUSTOM_PREFIX_SUPPORT) option(GLIBC_COMPATIBILITY "Enable compatibility with older glibc libraries." ON) option(USE_LIBCPP "Use libc++" OFF) option(USE_JEMALLOC "Use jemalloc" ON) +# Merge groups of .cpp files into jumbo translation units for much faster full +# builds. Turn OFF for precise per-file diagnostics, per-file tooling +# (clang-tidy/coverage), or the finest-grained incremental rebuilds. +option(ENABLE_UNITY_BUILD "Enable CMake unity builds for BE targets" ON) Review Comment: Fixed in 90d33da6061 — `report.py` maps a generated unity source back to its target's directory before grouping, so `be/build_<Type>_compile_bench/src/storage/CMakeFiles/Storage.dir/Unity/unity_0_cxx.cxx` now rolls up under `be/src/storage` instead of the build-tree bucket. The regex was checked against the paths a real configure generates for the six pilot batches. One limit worth stating rather than hiding: attribution is only as fine as the target's own directory. The Service batch merges just `be/src/service/http` sources but lands on `be/src/service`, and the Storage batches land on `be/src/storage` rather than `be/src/storage/index`. A unity TU is one timing for all of its members by construction, so second-level attribution inside a batch is not recoverable from the timing data — only from an `ENABLE_UNITY_BUILD=OFF` run. Fixture added in `build-support/tests/test-compile-bench-unity.sh` (unity path, ordinary source, and a non-unity build-tree path). -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
