github-actions[bot] commented on code in PR #66712:
URL: https://github.com/apache/doris/pull/66712#discussion_r3773989637
##########
be/src/information_schema/CMakeLists.txt:
##########
@@ -25,3 +25,13 @@ file(GLOB_RECURSE SRC_FILES CONFIGURE_DEPENDS *.cpp)
add_library(InformationSchema STATIC ${SRC_FILES})
pch_reuse(InformationSchema)
+
+# Unity build: these schema scanners are homogeneous glue code whose per-TU
+# cost is dominated by re-parsing the same header closure once per file.
+# Batch size 0 merges all sources into a single unity TU (~10x faster than
+# compiling them individually). A new file whose file-scope symbols clash
+# inside the unity TU can opt out via SKIP_UNITY_BUILD_INCLUSION.
+if (ENABLE_UNITY_BUILD)
Review Comment:
This false branch is not authoritative if the cache already has CMake's
standard `CMAKE_UNITY_BUILD=ON`. That variable initializes each target's
`UNITY_BUILD` property when `add_library` runs; with `ENABLE_UNITY_BUILD=OFF`,
this block simply leaves the inherited `ON` value untouched. I reproduced
`ENABLE_UNITY_BUILD=OFF`, `CMAKE_UNITY_BUILD=ON`, and `UNITY_BUILD=ON`, with a
database containing only a generated unity source. The same positive-only gate
appears in Service and Storage, so the advertised OFF escape hatch can fail on
a reused native-unity cache. Please explicitly set the pilot targets' property
OFF when this option is OFF.
##########
build.sh:
##########
@@ -943,6 +944,7 @@ if [[ "${BUILD_BE}" -eq 1 ]]; then
-DSTRIP_DEBUG_INFO="${STRIP_DEBUG_INFO}" \
-DDISPLAY_BUILD_TIME="${DISPLAY_BUILD_TIME}" \
-DENABLE_PCH="${ENABLE_PCH}" \
+ -DENABLE_UNITY_BUILD="${ENABLE_UNITY_BUILD:-ON}" \
Review Comment:
The new switch is only forwarded by `build.sh`. `run-be-ut.sh` has its own
clean CMake configure (`run-be-ut.sh:321-341`) and does not pass this value, so
`ENABLE_UNITY_BUILD=OFF ./run-be-ut.sh --clean ...` still recreates the cache
with this option's default `ON`. That means the advertised per-file escape
hatch cannot be used for the standard BE UT/UT-coverage workflow when
diagnosing a unity-only failure. Please mirror this plumbing and
effective-value logging in `run-be-ut.sh`.
##########
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:
Defaulting this option ON makes the standard compile-benchmark dependency
model silently incomplete. `cut_impact.py::load_tus()` accepts a Ninja
dependency block only when its first dependency is an original source under
`be/src` or `gensrc/build`; a unity block starts with a generated
`.../Unity/unity_N_cxx.cxx`, so it discards the whole block, including all
later original-source/header dependencies. The real loader retained the
standalone fixture but returned no TU for the equivalent unity shape.
Consequently `edge`/`audit`/`why` omit all six pilot batches (211 sources).
Please expand unity blocks or fail explicitly with a non-unity rebuild
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:
This default also breaks the compile benchmark's advertised per-directory
attribution for every pilot batch. The timing wrapper records the generated
source path
(`be/build_<Type>_compile_bench/src/<dir>/.../Unity/unity_N_cxx.cxx`), but
`report.py::group_keys()` recognizes logical modules only when a path starts
with `be/src`. With the real helpers, an InformationSchema source groups under
`be/src/information_schema`, while its unity source groups under `be` /
`be/build_Release_compile_bench`. The report therefore hides the optimized
segments' costs in a build-directory bucket. Please normalize unity paths to
logical target/source directories and add a report fixture.
--
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]
