morningman opened a new pull request, #66672: URL: https://github.com/apache/doris/pull/66672
> Split out of **https://github.com/apache/doris/pull/66510**, which carries the whole > BE build-time batch. This PR continues the header-closure surgery merged as > **#66400** with three more waves, and is independent of the rest of that batch — > it can be reviewed and merged on its own. ### What problem does this PR solve? Related PR: #66400, #66510 Problem Summary: Three waves of include-edge surgery in the BE header graph, each in the same "seed first, then cut" shape as #66400: one purely additive commit that outlines the inline bodies forcing the instantiation and seeds the direct includes the cut will expose, then one commit that deletes the edges and pins them with new `build-support/check-header-deps.py` rules. #### Wave 1 — three hot edges (`Prepare cutting three hot edges` + `Cut three hot include edges`) | edge cut | why it cost so much | how it is made removable | |---|---|---| | `core/pod_array.h` → `runtime/thread_context.h` | **dead include** left over from the PODArray memory-tracking experiment (#50549); the tracking logic has since moved into `Allocator` and `pod_array.h` names no `thread_context` symbol | just delete it — 203 TUs stop seeing `thread_context.h` and 195 of them stop seeing `exec_env.h` (1.37 MB/TU differential payload) | | `core/column/column.h` → `exec/sort/hybrid_sorter.h` | a `core` → `exec` layering violation reaching **808 TUs** | `HybridSorter` only appears in virtual signatures, so a forward declaration covers it; the `BE_TEST`-only `get_permutation_default` body (it constructs a `HybridSorter` by value) moves to `column.cpp` | | `core/wide_integer_impl.h` → `boost/multiprecision` (unconditional) | 4.27 MB of preprocessed closure in **1169 TUs** on every platform without an 80-bit long double | the from-double members stay inline (and `constexpr`) only where `LDBL_MANT_DIG == 64`; elsewhere they compile once in a new `core/wide_integer_from_double.cpp` with explicit instantiations for `integer<128\|256, signed\|unsigned>`. They were never `constexpr` on those platforms, so no constant evaluation is lost | Preprocessed closure of `column.h`: **16.72 MB → 10.15 MB (-39%)**. #### Wave 2 — two instantiation amplifiers: RLE and FMT_COMPILE Both parquet `decoder.h` trees included `util/rle_encoding.h` only because inline `BaseDictDecoder` bodies made ~530 TUs instantiate the `RleBatchDecoder<uint32_t>` → `GetLiteralValues` → `UnpackBatch` → `UnpackValues` chain — measured at **0.43 CPU s per TU, 231 CPU s total** — on top of reparsing the 2038-line rle/bit-stream/bit-packing family each time. The eight per-page / per-batch methods plus the ctor and dtor move out of line, so the `unique_ptr<RleBatchDecoder<uint32_t>>` member only needs the complete type in `decoder.cpp`. Everything moved is dispatched through the vtable at every call site already, so no generated call changes; the real decode TUs keep including `rle_encoding.h` directly and inline the chain exactly as before, and the per-value-hot `LevelDecoder::get_next` path is deliberately untouched. `core/uint24.h` and `core/value/large_int_value.h` pushed their `FMT_COMPILE` formatter instantiations (the `"{:04d}-{:02d}-{:02d}"` date formatter is the single biggest fmt instantiation in the tree, plus the int128 formatters) into ~1150 TUs, **53.5 CPU s**. Those bodies move to `.cpp` files and both headers drop `<fmt/compile.h>` / `<fmt/format.h>`. They return `std::string` and are allocation-dominated, so the now-outlined call is noise. #### Wave 3 — the DataVariants amplifier behind `dependency.h` `exec/pipeline/dependency.h` and `exec/pipeline/rec_cte_shared_state.h` hold non-template **in-class inline** bodies — SharedState constructors, destructors, close paths and three `std::visit` dispatches — that name the full `AggregatedDataVariants` / `JoinDataVariants` / `SetDataVariants` / `DistinctDataVariants` surface. Such bodies are semantically analyzed **when the header is parsed**, not when they are called, so every one of the ~128 TUs that transitively include `dependency.h` instantiated that whole surface at a flat **~0.85 CPU s per TU (~130 CPU s)** — and 105 of those TUs never touch a variant. All of it is per-query setup/teardown, so it moves to `dependency.cpp` and a new `rec_cte_shared_state.cpp`. With the bodies gone the headers can drop: - `exec/common/agg_utils.h`, `set_utils.h`, `distinct_agg_utils.h` — forward declarations suffice; - `exec/common/join_utils.h` → the new light `exec/common/join_op_utils.h` (JoinOpVariants and the `AsofIndexGroup` family split out of `join_utils.h`, which re-exports them; `dependency.h` holds these **by value**, and the new header depends only on thrift enums, pdqsort and std containers — not on the hash tables); - `exec/operator/join/process_hash_table_probe.h` — dead include; - `util/brpc_closure.h` — dead include, and the sole route carrying `query_context.h`, `thread_context.h` and `service/brpc.h` into ~100 pipeline TUs (1.36 MB/TU); - `<concurrentqueue.h>` — dead include (152 KB third-party single header); - `util/brpc_client_cache.h` from `rec_cte_shared_state.h` — the rpc send bodies live in the `.cpp` now. `BucketedAggSharedState::init_instances` also becomes a non-template taking `std::function`: non-dependent constructs in a member-template body are checked at definition time, so the old inline template forced the destructor of `unique_ptr<BucketedAggDataVariants>` on every includer despite never being called there. #### One CI-critical commit rides along `Opt wide_integer_from_double.cpp out of the PCH` must ship **in this PR**, not as a follow-up. Upstream's clang toolchain defaults to `ENABLE_PCH=ON` (`be/CMakeLists.txt`), and `pch.h` transitively includes `wide_integer_impl.h`, whose include guard is then already consumed when the impl TU is compiled — the explicit instantiations would find no definition. It is the only file in the tree with this interaction, but splitting the two commits apart would leave a state that fails the clang CI lane. ### Benefit Compile-time only; no runtime behavior change. | wave | effect | measurement | |---|---|---| | W1 | **-24.3 s** wall, `be/src` CPU **-3.6%** | same-codebase A/B on the batch branch | | W2 | **-125 s / -6.9%** (30m24s → 28m19s) | same-codebase A/B, back-to-back | | W3 | ~130 CPU s of parse-time instantiation removed (~0.85 CPU s × ~128 TUs), plus 1.36 MB/TU × ~100 TUs of dead brpc payload | P3.5 research traces (per-TU CPU, not end-to-end wall) | Roughly **-2.5 min** off the cold build in total. **Measurement conditions, stated plainly:** these were taken on the batch branch on macOS/arm64 + clang 20 at `-j5`/`-j6` with the repo's `--compile-bench` harness, each wave as a same-codebase A/B pair rather than one end-to-end run of this PR alone. W3's number is per-TU CPU from the instantiation traces, not a wall-clock A/B. Absolute numbers on the CI machines will differ; the mechanism (fewer TUs parsing and instantiating the same templates) does not. ### Risk and verification - **Header sweeps.** Every wave was validated with a full `-fsyntax-only` sweep over the natural (no-PCH) include closure: W1 1358 TUs with only the 4 known pre-existing failures, W2 **1364/1364 clean**, W3 **0 failing of 1365**. - **BE unit tests.** The first sweeps covered `be/src` only, so a full `ninja -k 0` over 2422 targets was run to reach the 1024 `be/test` TUs; it surfaced 8 failing TUs in 4 families, all repaired in `Repair the BE UT build after the include-edge cuts`. **Nothing there restores a cut edge and no production header gains an include.** Two of those repairs fix a latent defect that predates this PR: `BaseDictDecoder`'s defaulted-in-class constructor odr-uses `~unique_ptr<RleBatchDecoder<uint32_t>>` in every TU constructing a derived decoder, so the header's own claim that the complete type is only needed in `decoder.cpp` did not hold — in **both** trees, though only the `format/` one had a test reaching it. - **Rebased onto current master (`c29075a7e10`) and rebuilt from scratch** in a clean worktree, macOS/arm64 + clang 20, `ENABLE_PCH=ON` (upstream's clang default, so the PCH opt-out above is exercised): **8554/8554 ninja edges, zero failures, `doris_be` links.** `compile_commands.json` confirms `wide_integer_from_double.cpp` is the one first-party TU compiled without the PCH. - **BE unit tests rebuilt on the rebased tree: all 9602 objects compile, zero failures**, and the final link resolves every symbol (0 undefined). Two pre-existing macOS-only obstacles were hit on the way and are called out under *Proactive disclosure*. - **Runtime cost of outlining.** Everything moved out of line is either per-query setup/teardown (SharedState ctor/dtor/close), or a per-page/per-batch method already reached through a vtable. No per-value or per-row hot path was outlined. - **Guard rules.** `check-header-deps.py` gains rules pinning `pod_array.h !-> thread_context.h`, `column.h !-> exec/sort/`, and both `decoder.h` headers `!-> util/rle_encoding.h` (19 rules total, all passing). ### Proactive disclosure - **One commit here is unrelated to include edges: `Make hierarchical_data_iterator_test compile on macOS arm64`.** `std::min(*rows, ROWS - current_ordinal)` cannot deduce `_Tp` where `size_t` is `unsigned long` and `ordinal_t` (`uint64_t`) is `unsigned long long`, which is the case on macOS/arm64 but not on Linux — so CI is green while `be/test` does not build on macOS at all. It arrived with #66204. It is fixed here because this branch is verified on macOS and the break blocks that verification; happy to split it out if a reviewer prefers. - **The macOS Debug UT link has outgrown the Mach-O format**, independently of this PR: `section __debug_names's file offset exceeds 4GB`. Master's own test growth is what crossed the line — the same worktree layout linked fine on 2026-08-10 at 7.6 GB of test debug info, and master is now at 8.0 GB across 10 more test TUs, while this PR adds one `#include` to each of 5 test files. Omitting the debug map (`-Wl,-S`) links the binary cleanly with zero undefined symbols, which is how the link was verified here. Worth someone's attention as a separate issue. - **Cross-platform is the blind spot.** All measurements and all sweeps ran on macOS/arm64 + clang 20. Nothing here is platform-specific by construction, but the Linux gcc/clang lanes are covered only by upstream CI, not by any local gate — please give those two lanes a look. - **Two cuts have no scanner rule.** The boost/multiprecision edge is preprocessor-gated and `check-header-deps.py` is preprocessor-blind (it would flag the impl TU's gated include); the macro structure is self-guarding instead — breaking it fails the impl TU's build. The fmt cuts are likewise only noted in a comment, since the scanner follows quoted project includes. - **Three test files carry `clang-format off/on` around their includes.** `asof_join_test` and the two `fix_length_dict_decoder` tests are include-order-sensitive: the supplying include has to come *before* the header under test (ADL cannot reach the global `pdqsort` from `std::vector`'s iterators; a `unique_ptr<RleBatchDecoder<uint32_t>>` dereference depends on no template parameter, so it binds where the template is parsed). Without the marker the formatter sorts the include back and breaks the build. The cost was deliberately kept in the tests rather than paid by a production header. - **`DISALLOW_COPY_AND_ASSIGN` in `storage/olap_define.h` loses its trailing semicolon.** `butil/macros.h` defines the same macro without one and wins under `#ifndef` in TUs that see butil first, so the two expansions have to stay call-site compatible. All 45 call sites in the tree already write the `;`. ### 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 UT build + link, and the `-fsyntax-only` sweeps above) - 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]
