morningman opened a new pull request, #66712: URL: https://github.com/apache/doris/pull/66712
> 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` exactly like `ENABLE_PCH`, overridable from the environment / `custom_env.sh`. Every per-target `UNITY_BUILD` property is gated on it from day one. 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`. 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. - **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**. ### Release note None ### Check List (For Author) - Test - [x] No need to test or manual test. Explain why: - [x] This is a refactor/code format and no logic has been changed. - [x] Previous test can cover this change. (full BE build + BE UT builds on the development branch; symbol-parity checks per target) - Behavior changed: - [x] No. - Does this need documentation? - [x] No. -- 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]
