morningman opened a new pull request, #66776: URL: https://github.com/apache/doris/pull/66776
> 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. This PR extends unity to **Exec and > Exprs** — the two heaviest targets in the BE build and the largest single source of > the unity line's win. The remaining targets follow in one more PR. ### What problem does this PR solve? Related PR: #66510, #66712 Problem Summary: Same mechanism as #66712: 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. What is new here is the scale — Exec (174 TUs, `libExec.a` 2450 MB) and Exprs (287 TUs, `libExprs.a` 2721 MB) are the two heaviest targets in the tree, and their glue shares the heaviest closures (`operator.h`/`dependency.h` for Exec; the vexpr/factory closure for Exprs). The four commits: 1. **Deduplicate exec file-scope names that clash under unity** (no behavior change): `file_scanner.cpp`/`file_scanner_v2.cpp` both defined the Iceberg delete content codes and `is_iceberg_position_deletes_sys_table()` in anonymous namespaces — the shared trio moves to `iceberg_scan_semantics.h` (`file_scanner_v2_test.cpp` carried a third copy, kept file-local by #66615 because this header move had not landed yet; it now uses the header too). `vtablet_writer.cpp`/`vtablet_writer_v2.cpp` both defined a file-scope `CLOSE_WAIT_EVENT_FALLBACK_MS` — scoped into `IndexChannel` and `VTabletWriterV2`; v2's file-scope `on_partitions_created()` trampoline renamed `on_partitions_created_v2` (the two functions cast to different writer types). `exchange_sink_operator.cpp`'s namespace-scope `timer_name` renamed `wait_for_dependency_timer_name` (shadowed unity siblings' locals under `-Wshadow -Werror`). 2. **Unity for the whole Exec target**: 167 of 174 TUs join 14 unity batches of ≤12 sources, gated on `ENABLE_UNITY_BUILD` like the pilot targets. Opted out: five files whose file-scope macros must not leak into siblings, plus the two heaviest template-instantiation TUs (`operator.cpp`, `hashjoin_build_sink.cpp`) which would dominate any batch they join; `scan_operator.cpp` is both. 3. **Two latent defects the Exprs conversion surfaced** (stand on their own): `dictionary_factory.h` had **no include guard at all** — any TU reaching it through two include paths fails with a class redefinition, and under unity the clang error recovery poisoned unrelated batch members with spurious `-Warray-bounds` diagnostics. Now `#pragma once`. And `function_dict_get_many.cpp` had copy-pasted the `DictGetState` struct from `function_dict_get.cpp` at namespace scope — renamed `DictGetManyState` so the two TUs can share a batch. 4. **Unity for the Exprs glue**: 249 of 287 TUs join 32 unity batches of ≤8 sources, same gate. Opted out: the flex/bison/gperf generated tables, seven macro-leaking files, and the 30 heavy template-instantiation TUs (>15 s wall or >2.2 GB RSS in the compile bench: the min_max/collect/topn/percentile aggregate family, `in.cpp`, `multiply.cpp`, `function_array_aggregation.cpp`, …) whose per-file codegen would only stack into jumbo poles. ### Measured results All numbers from the development branch this series is split from, macOS arm64 + clang 20, `-j14`, `ENABLE_PCH=ON`, cold builds, back-to-back A/B. The baseline is the #66712 state of that branch (10m16s), so the two waves compose with the pilot: | wave | build phase wall | target slot time | archive size | |---|---|---|---| | Exec unity | 10m16s → **8m57s (-79.2 s / -12.9%)** | 1533 s → 579 s (2.6×) | `libExec.a` 2450 MB → 641 MB | | Exprs unity | 8m57s → **7m57s (-60.0 s / -11.2%)** | 2147 s → 1333 s | `libExprs.a` 2721 MB → 1392 MB | Jumbo-TU envelope: the largest Exec unity TU compiles in 32 s / 2.6 GB RSS standalone, the largest Exprs one in 25 s / 1.95 GB — both 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.** The code commits riding along are hygiene: constants deduplicated with identical values, one constant scoped into its class, two renames, one `#pragma once`. No logic change. - **Archive symbol parity** (checked on the development branch): Exec keeps all external defined symbols — three weak linkonce_odr template instantiations dedup away, which is the point of unity, not a loss. Exprs likewise (one weak instantiation dedups; the `DictGetManyState` rename carries its `shared_ptr` machinery under the new name). - **This exact branch, rebased onto current master, full BE build from scratch** (macOS arm64, clang 20, `ENABLE_PCH=ON`, `ENABLE_UNITY_BUILD=ON`): **7981/7981 ninja edges, zero failures, `doris_be` links (325 MB).** Exec produces exactly 14 unity TUs and Exprs exactly 32, as advertised. This includes `pipeline/rec_cte_shared_state.cpp`, added upstream after the waves were measured — it lands inside an Exec unity batch via the existing `GLOB_RECURSE` with zero CMakeLists edits, which is the intended maintenance story. - **The OFF path, checked on the same tree**: reconfiguring with `ENABLE_UNITY_BUILD=OFF` drops **all 52** `unity_*.cxx` entries from `compile_commands.json` (Exec 14, Exprs 32, the #66712 pilots 6) and the TU count goes 8462 → 9042 — the batches return to exactly their 632 member files. Reconfiguring back ON restores exactly the same 52 batches. The switch semantics themselves (including winning over a stale `CMAKE_UNITY_BUILD` cache) were established in #66712. - **These two targets 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). - The `file_scanner_v2_test.cpp` hunk was compile-verified standalone against this branch (`-fsyntax-only` with the test TU's full include closure). ### Proactive disclosure - **Cross-platform is the blind spot, closed by this PR's own CI** — every local build and measurement above is macOS arm64 + clang 20. With `ENABLE_UNITY_BUILD` defaulting ON since #66712, the Linux compile lanes and every regression pipeline in this PR's CI run against unity Exec/Exprs — that is the validation. Please give the Linux gcc lane in particular a look. Escape hatches, in order: per-user `ENABLE_UNITY_BUILD=OFF`, per-file `SKIP_UNITY_BUILD_INCLUSION`, or a one-line default flip. - **The incremental-rebuild trade-off is real**: touching one `.cpp` inside a batch recompiles the whole batch (≤12 sources for Exec, ≤8 for Exprs; a batch compiles in ~25–32 s). This is why the heaviest, most-edited TUs (`operator.cpp`, `hashjoin_build_sink.cpp`, the aggregate families, `in.cpp`, `multiply.cpp`, …) are deliberately SKIPped and keep per-file granularity, and `ENABLE_UNITY_BUILD=OFF` restores it everywhere. - **The SKIP lists are coverage policy, not leftovers**: 7 Exec + 38 Exprs files stay individual on purpose — generated parsers (flex/bison/gperf), files whose file-scope macros would leak into siblings, and the heavy codegen TUs where merging saves no closure parse worth the jumbo-TU cost. A future file whose file-scope symbols clash inside a unity TU opts out the same one-line way. - Unity covers the *glue* of these targets, not the codegen-heavy families — the 30 heavy Exprs SKIPs mean the headline per-target ratios (2.6× Exec slot time) are earned on the batched part; the SKIPped monsters keep their cost and their per-file granularity. ### 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; per-target archive symbol-parity checks) - Behavior changed: - [x] No. - 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]
