mirror of
https://github.com/apache/doris.git
synced 2026-11-04 07:31:17 +00:00
[opt](build) Add ENABLE_UNITY_BUILD and pilot unity builds on three glue targets (#66712)
> Part of the BE build-time optimization series tracked in #66715. > > Split out of **https://github.com/apache/doris/pull/66510**, which carries the whole > BE build-time batch. After the header-closure surgery in **#66400** and **#66672**, > this PR opens the second line of that batch: CMake unity builds. It adds the > infrastructure switch and converts the three lowest-risk glue segments as a pilot; > the heavier targets (Exec, Exprs, and the rest) follow in separate PRs once this > one has proven the mechanism cross-platform. ### What problem does this PR solve? Related PR: #66510, #66672 Problem Summary: Most of the BE's cold-build time is not spent compiling our code — it is spent **re-parsing the same shared header closure once per small `.cpp`**. For the glue directories this ratio is extreme: the 51 InformationSchema scanners cost ~124 CPU s (with PCH) of which almost everything is the closure re-parse; the ~58 http handlers sum to ~5.6 min of slot time for a few thousand lines of handler logic. CMake's built-in `UNITY_BUILD` (CMake ≥ 3.16; we require 3.19.2) concatenates groups of `.cpp` files into jumbo TUs, so a closure is parsed **once per batch** instead of once per file. This PR: 1. **Adds the switch** — `option(ENABLE_UNITY_BUILD ON)` in `be/CMakeLists.txt`, plumbed through `build.sh` and `run-be-ut.sh` exactly like `ENABLE_PCH`, overridable from the environment / `custom_env.sh`. Every per-target `UNITY_BUILD` property is set unconditionally from it -- including an explicit OFF, so the switch also wins over CMake's own `CMAKE_UNITY_BUILD` in a reused cache. Turn it OFF for per-file diagnostics, per-file tooling (clang-tidy, coverage), or the finest-grained incremental rebuilds. 2. **Pilots unity on three glue segments** (and only there — everything else is explicitly opted out per file, not by omission): - **InformationSchema**: all 51 scanner TUs → 1 unity TU (`UNITY_BUILD_BATCH_SIZE 0`). - **Service, scoped to `http/`**: ~58 handler TUs → 1 unity TU. The non-http service sources (service entry points, arrow_flight) are heterogeneous heavy TUs that gain nothing from merging and stay individual. - **Storage, scoped to `index/`**: ~111 TUs sharing one CLucene-heavy closure → 4 unity TUs of ≤ 32 sources (batch size bounds jumbo-TU size and memory). 3. **Makes the merged TUs legal C++** with two hygiene commits that stand on their own even without unity: - 27 http action files each carried a private copy of the same file-scope constants (`HEADER_JSON` ×16, `TABLET_ID` ×7, `SCHEMA_HASH` ×4, …). They move to one shared header as C++17 inline variables, values unchanged. - `prefix_query` declared `get_prefix_terms(IndexReader*)` relying on a header-level `CL_NS_USE(index)` using-directive. Inside `doris::segment_v2` that unqualified name silently flips to `doris::segment_v2::IndexReader` as soon as any sibling source brings that type into scope — a landmine with or without unity. The declaration now spells out `lucene::index::IndexReader`. 4. **Fixes a latent bug unity exposed**: `schema_scanner_helper.h` opened with `#ifndef _SCHEMA_SCANNER_HELPER_H_` but never defined the macro (and one include sat outside the guard), so the guard never worked. Harmless while every TU included it exactly once; breaks immediately under unity. Now `#pragma once`. 5. **Keeps the compile-bench tooling honest under unity** — with the default ON, `build.sh --compile-bench` produces a build directory whose sources are jumbo TUs, which two of the already-merged tools silently mis-read: `cut_impact.py` dropped every unity dependency block (and with it all 211 pilot sources) from its TU set, and `report.py` filed each batch's time under the build directory instead of the module it came from. `cut_impact.py` now refuses a unity build directory with the rebuild instruction — its analysis needs per-source closures, and splitting a batch's union back over its members would over-estimate instead of under-report — `report.py` attributes a unity TU to its target's directory, and `rebuild_radius.py` says when its per-header counts are batch-granular. `build-support/tests/test-compile-bench-unity.sh` pins all three behaviours. Files whose file-scope macros must not leak into unity siblings stay individual via `SKIP_UNITY_BUILD_INCLUSION`: `http_parser.cpp` (CR/LF), `be_thread_stack_action.cpp` (`UNW_LOCAL_ONLY`), and four index files (`CL_MAX_PATH` and friends, `IS_CHINESE_CHAR`, `APPLY_FOR_PRIMITITYPE`). A future file whose file-scope symbols clash inside a unity TU can opt out the same way. ### Measured results All numbers from the development branch this series is split from (which also carried the #66672 include cuts), macOS arm64 + clang 20, `-j14`, `ENABLE_PCH=ON`, cold builds, back-to-back A/B: | metric | before | after | |---|---|---| | build phase wall | 11m38s | **10m16s (-82.4 s / -11.8%)** | | Σ per-TU CPU | 147.9 min | **134.8 min (-8.9%)** | | TU count | 8384 | 8180 | | failures | 0 | 0 | Per-segment slot time (sum of compile time the segment occupies across slots): | segment | before | after | ratio | |---|---|---|---| | information_schema | 221.7 s | 17.0 s | **13×** | | service/http | 273.6 s | 26.3 s | **10.4×** | | storage/index | 264.2 s | 120.1 s | **2.2×** (remainder includes the four macro SKIPs) | Why unity rather than more PCH: a serial cold micro-benchmark of the InformationSchema target alone measured **9.9× with PCH** (124 s → 12.5 s) and **14.6× without PCH** (206 s → 14.1 s) — and unity *without* PCH still beats individual TUs *with* PCH by 8.8×. Unity removes the repeated parse instead of amortizing it, and the two compose. Side effects on artifacts: `libInformationSchema.a` 334 MB → 29 MB, `libStorage.a` 1492 MB → 1318 MB (linkonce_odr instantiations dedup inside each unity TU). The largest unity TU peaks at 2.1 GB RSS — *below* the largest existing individual TU in the tree (3.9 GB), so `-jN` memory envelopes are unchanged. ### Risk and verification - **Unity changes TU grouping only; no code changes ride along** beyond the two hygiene commits described above (constant dedup with identical values, one qualified name, one include guard). - **Archive symbol parity** was checked per target on the development branch: InformationSchema keeps all 2303 external defined symbols (plus 5 weak `unique_ptr<SchemaXxxScanner>` instantiations that dedup), Service keeps all 4054, Storage keeps all external symbols with 8 weak linkonce_odr template instantiations deduping away — which is the point of unity, not a loss. - **This exact branch, rebased onto current master, full BE build from scratch** (macOS arm64, clang 20, `ENABLE_PCH=ON`, `ENABLE_UNITY_BUILD=ON`): **8349/8349 ninja edges, zero failures, `doris_be` links.** The six expected unity TUs (InformationSchema ×1, Service http ×1, storage/index ×4) all compile. This includes `schema_tso_status_scanner.cpp`, added upstream after the pilot was measured — it lands inside the InformationSchema unity TU via the existing `GLOB_RECURSE` with zero CMakeLists edits, which is the intended maintenance story. - **The OFF path is verified on the same tree**: reconfiguring with `ENABLE_UNITY_BUILD=OFF` removes every `unity_*.cxx` entry from `compile_commands.json` and flips **exactly 219 ninja edges** — the three targets' per-file objects plus their archives and the final link, nothing else. All of them compile per-file with zero failures and `doris_be` links again. The blast radius of the switch is precisely the three pilot targets. - **`OFF` also wins over a native-unity cache**: configuring with `-DENABLE_UNITY_BUILD=OFF -DCMAKE_UNITY_BUILD=ON` produced 28 unity TUs across the three pilot targets before the gate fix and **0** after, while the ordinary `-DENABLE_UNITY_BUILD=ON` configure still produces exactly the 6 advertised batches (InformationSchema ×1, Service ×1, Storage ×4). - **These three segments have been building as unity TUs on the development branch since 2026-08-08**, through repeated full-tree builds and the BE UT builds that verified #66672 (the UT binaries link against these same target libraries). ### Proactive disclosure - **Cross-platform is the blind spot, and the default is deliberately ON so this PR's own CI closes it.** Every local build and measurement above is macOS arm64 + clang 20. Nothing here is platform-specific by construction, but with the default ON, the Linux compile lanes and every regression pipeline in this PR's CI run against unity builds — that is the validation. Please give the Linux gcc lane in particular a look. If some environment trips over unity after merge, the escape hatches are, in order: per-user `ENABLE_UNITY_BUILD=OFF` (env or `custom_env.sh`), per-file `SKIP_UNITY_BUILD_INCLUSION`, or a one-line default flip — the infrastructure stays either way. - **The incremental-rebuild trade-off is real**: touching one `.cpp` inside a unity batch recompiles the whole batch. For the glue chosen here a batch compiles in ~16–30 s, comparable to single mid-weight TUs elsewhere in the tree; and `ENABLE_UNITY_BUILD=OFF` restores per-file granularity for workflows that need it. This is also why the pilot targets are glue directories and not the hot-edit paths. - **contrib (openblas/clucene) was evaluated and deliberately left alone**: the f2c-generated LAPACK sources and the snowball stemmers define clashing file-scope statics (`static c__1` and friends) — structurally un-unifiable without rewriting generated code — and contrib compiles once and rarely changes. - **The rest of Storage and Service is opted out per file, on purpose.** Merging heavy heterogeneous TUs earns nothing (the closure parse is not the dominant cost there) and risks monster TUs. Follow-up PRs extend unity to Exec, Exprs and the remaining targets with the same SKIP discipline; on the development branch the full rollout takes the same tree from 10m16s to **6m18s**.
This commit is contained in:
1 parent
8f531491f1
commit
7b00434015
41 files changed
+296
-67
No files matched your search
@@ -223,6 +223,7 @@ echo "Get params:
|
||||
PARALLEL -- ${PARALLEL}
|
||||
CLEAN -- ${CLEAN}
|
||||
ENABLE_PCH -- ${ENABLE_PCH}
|
||||
ENABLE_UNITY_BUILD -- ${ENABLE_UNITY_BUILD:-ON}
|
||||
EXTRA_BE_MODULES -- ${EXTRA_BE_MODULES}
|
||||
"
|
||||
echo "Build Backend UT"
|
||||
@@ -335,6 +336,7 @@ cd "${CMAKE_BUILD_DIR}"
|
||||
${CMAKE_USE_CCACHE_CXX:+${CMAKE_USE_CCACHE_CXX}} \
|
||||
${CMAKE_USE_CCACHE_C:+${CMAKE_USE_CCACHE_C}} \
|
||||
-DENABLE_PCH="${ENABLE_PCH}" \
|
||||
-DENABLE_UNITY_BUILD="${ENABLE_UNITY_BUILD:-ON}" \
|
||||
-DDORIS_JAVA_HOME="${JAVA_HOME}" \
|
||||
-DBUILD_AZURE="${BUILD_AZURE}" \
|
||||
"${BE_EXTRA_CMAKE_ARGS[@]}" \
|
||||
|
||||
Reference in new issue
Block a user