morningman opened a new pull request, #66789: URL: https://github.com/apache/doris/pull/66789
> 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. #66712 introduced the `ENABLE_UNITY_BUILD` switch and piloted > unity builds on three low-risk glue targets; #66776 extended it to Exec and Exprs. > This PR finishes the line: unity builds for every remaining BE target where it > helps, plus a fail-loud guard for the skip lists. It is the last PR of the unity > sub-series (the remaining two PRs of the batch are orthogonal: extern-template > convergence and olap_common decoupling). ### What problem does this PR solve? Related PR: #66510, #66712, #66776 Problem Summary: Same mechanism as #66712/#66776: most of the cold-build cost of glue-heavy targets is **re-parsing the shared header closure once per small `.cpp`**, and CMake's `UNITY_BUILD` makes a batch pay that parse once. This PR applies it to the twelve remaining targets where the win is real, and writes down (in-tree) the two places where it is not. The eleven commits, in fix-then-enable pairs: 1. **Deduplicate cloud/storage file-scope names that clash under unity** — identical copies hoisted to one shared definition (warm-up metric window spans -> `bvar_windowed_adder.h`; the "D0R1" segment footer magic -> `segment/common.h`; the `path_allocated_bytes` PathInData accounting helper that #66204 landed in two variant v2 TUs -> `util/json/path_in_data.h`), a divergent copy renamed instead (`create_ext_meta_io_context` keeps file-cache stats where its siblings reset them); **plus one real latent bug**: `merger.cpp` threw with `INTERNAL_ERROR` bound to `PPlanFragmentCancelReason::INTERNAL_ERROR` (=3) from `types.pb.h` instead of `ErrorCode::INTERNAL_ERROR` (=6) — no `ErrorCode` using-directive was in scope. Qualified; those compaction-failure statuses now carry the intended code. 2. **Unity for Cloud (batch 12) and Storage target-wide (batch 12)** — Storage's previous index/-only inverse-selection flips to graded opt-outs. Cloud: 37 members -> 4 batches. Storage: 232 members -> 20 batches. 3. **Deduplicate format_v2 file-scope helpers** that clash under unity. 4. **Unity for the format_v2 half of Format (batch 8)** — the v1 `format/` tree is slated for removal, so every v1 file stays individual rather than paying the v1/v2 twin-symbol dedup a joint unity would need. 5. **Resolve runtime/load/io/util file-scope clashes** — including two more latent bugs found by merged TUs: `column_predicate.h` had the same unqualified `INTERNAL_ERROR` mis-binding as `merger.cpp`, and `vdatetime_value.h` defined a `static RE2` in the header (one RE2 construction per including TU at startup; now a C++17 inline variable, one program-wide). `runtime_profile.h` also carried two never-referenced private fields; removed. 6. **Unity for Runtime, Load, IO and Util** (batch 12/12/16/24 by member weight). 7. **Add missing include guards to two more headers** (`cgroup_cpu_ctl.h`, `complex_type_deserialize_util.h`) — found the same way as the two in #66776. 8. **Unity for Core, Agent, Udf, Common, ann_index** — and the Service negative result recorded in-tree (see below). 9. **Unity for the thrift half of DorisGen (batch 8)** — protobuf `.cc` files stay individual structurally: protoc emits identical file-scope statics (`schemas[]`, `file_default_instances[]`) in every one. 10. **Evict `file_cache_lru_tool.cpp` from `libIO.a`** — a standalone tool with its own `main()` must not be an archive member once unity can merge it into an object the `doris_be` link actually pulls in. 11. **Fail the configure when a unity skip entry goes stale** — `set_source_files_properties()` silently ignores nonexistent paths, so a rename would quietly dissolve a skip entry and the file would rejoin its batch with no diagnostic. All thirteen skip lists (including the merged Exec/Exprs and pilot ones) now go through `doris_skip_unity_inclusion()`, which FATAL_ERRORs on a nonexistent entry (generated `GENSRC_DIR` paths exempt — they legitimately do not exist on a fresh configure). ### Benefit Measured on the development branch this series is split from (arm64 macOS, clang 20, `-j14`, PCH on, cold builds, same-machine paired runs): - this wave (on top of the #66776 state): **7m57s -> 6m18s (-99s / -20.7%)**, sum-of-TU CPU (user) 98.7 -> 76.7 min, compile units 7810 -> 7280. - static archives collapse as duplicate DWARF / weak-template copies merge (measured on this branch, unity OFF vs ON): libStorage.a 1592 -> 514 MB, libCloud.a 441 -> 107 MB, libRuntime.a 356 -> 122 MB, libLoad.a 311 -> 100 MB, libFormat.a 1057 -> 760 MB, libCore.a 371 -> 182 MB. Less disk thrash per build, faster archiving and linking. - the four latent bugs above found and fixed (two of them wrong-code). ### Verification on this branch (arm64 macOS, clang 20, ENABLE_PCH=ON) - unity=ON full build: zero failures across all 7927 TUs, `doris_be` links (319 MB). One upstream-drift clash surfaced and fixed first (the #66204 `path_allocated_bytes` twins — folded into the dedup commit). - unity coverage after this PR: 110 batches / 1224 member files tree-wide, of which this PR adds 63 batches / 696 members (per-target counts in the commit messages); `compile_commands.json` agrees. - `ENABLE_UNITY_BUILD=OFF` reconfigure: unity entries drop to zero and the TU count goes 7927 -> 9041 = exactly the 110 batches dissolving into their 1224 members; full rebuild green, `doris_be` relinks -> the switch's blast radius is precisely the unity config. - BE UT (`BUILD_TYPE_UT=Debug`): doris_be_test compiles and links clean — this specifically exercises the four "a test #includes the src .cpp" opt-outs below. - fail-loud helper: deliberately renaming one skip entry makes the configure fail with `unity skip entry does not exist (renamed or moved?): <path>` and the offending CMakeLists line; reverting restores a clean configure. ### Deliberate opt-outs (not omissions) - **Per-file opt-outs** (~50 files across the targets, each annotated in its CMakeLists): file-scope macro leakers, the heaviest template-instantiation TUs (so no batch inherits their weight), `ENABLE_VARIANT_NESTED_GROUP` / `ENABLE_TDE` module swaps (unaudited out-of-tree sources), and generated protobuf `.cc`. - **Four files a test compiles a second time by `#include`-ing the .cpp** (`storage/compaction/collection_statistics.cpp`, `runtime/descriptors.cpp`, `core/column/column_variant.cpp`, `core/data_type/convert_field_to_type.cpp`): the test object must shadow a never-pulled archive member, but a unity batch is pulled in for its siblings and the linker sees a duplicate definition. Same failure class the first CI round of #66776 hit; this time the whole tree was swept up front (`grep -rn '#include ".*\.cpp"' be/test`) and the remaining hits are either already opted out, structurally outside unity scope, or test-side files. - **Service stays at http-only unity**: widening to the whole target at batch 8 was tried and measured **slower** (service segment slot time 104s -> 132s) — splitting the single http jumbo repays the shared-closure parse per batch and the heterogeneous non-http TUs gain too little to cover it. The CMakeLists comment records this so nobody retries it blind. - **contrib (openblas/clucene) deliberately untouched**: openblas is f2c output — 2055 files with colliding `static c__1` file-scope constants, structurally un-unifiable. ### Cross-platform note (please watch the gcc pipeline) All local verification is macOS/clang. gcc-only hazards were audited up front: the `-Wsubobject-linkage` pattern that bit #66776 (an anonymous-namespace type as a member of an external-linkage class turns into an error only once unity makes the file `#include`-d) was swept across all 627 files entering unity scope — every anonymous-namespace type usage found is function-local, so no hazard — but the sweep is heuristic, so the first Performance (gcc) CI round is worth watching. ### Release note None ### Check List (For Author) - Test - [x] Manual test (add detailed scripts or steps below) - Full BE build with unity ON: all 7927 TUs green, `doris_be` links. - `ENABLE_UNITY_BUILD=OFF` reconfigure + full rebuild green; TU accounting closes exactly (7927 -> 9041); flip back ON restores. - BE UT (`BUILD_TYPE_UT=Debug`): doris_be_test compiles and links with zero duplicate symbols. - Deliberately breaking a unity skip entry fails the configure with the new fail-loud diagnostic; reverting restores a clean configure. - Behavior changed: - [x] Yes. Two latent wrong-code fixes surfaced by unity merges (details in the commit messages): `merger.cpp` and `column_predicate.h` threw with `INTERNAL_ERROR` accidentally bound to `PPlanFragmentCancelReason` (=3); they now carry `ErrorCode::INTERNAL_ERROR` as intended. Also `vdatetime_value.h`'s header-defined `static RE2` became a C++17 inline variable: one instance program-wide instead of one per including TU (same matching behavior, less startup work and memory). - Does this need documentation? - [x] No. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01Gdfkk7RqgD5e3Uv7bTM3NV -- 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]
