[opt](build) Cut three more waves of hot include edges in the BE header graph - #66672
Open
morningman wants to merge 11 commits into
Open
[opt](build) Cut three more waves of hot include edges in the BE header graph#66672morningman wants to merge 11 commits into
morningman wants to merge 11 commits into
Conversation
Pure-additive / behavior-preserving preparation so that three include edges can be cut next: core/pod_array.h -> runtime/thread_context.h (dead include), core/column/column.h -> exec/sort/hybrid_sorter.h (core->exec layering violation reaching 808 TUs) and wide_integer_impl.h's unconditional boost/multiprecision include (4.27MB of preprocessed closure in 1169 TUs on platforms without an 80-bit long double). No include edge is removed here: - core/wide_integer_from_double.cpp (new): compiles the from-double conversion once and explicitly instantiates it for integer<128|256, signed|unsigned>; harmless while the header still defines the members inline - column.h: forward-declare HybridSorter; move the BE_TEST-only get_permutation_default body to column.cpp (it constructs HybridSorter by value); the production get_permutation default keeps its inline throw body - column_decimal.h: move the permutation<U> helper body to column_decimal.cpp (its only caller, get_permutation, is already defined there); member templates are instantiated by that call, not by the class-level explicit instantiations - column_const.h: include <span> directly (rode in via hybrid_sorter.h) - storage/olap_define.h: drop the trailing semicolon inside DISALLOW_COPY_AND_ASSIGN so the call sites' `;` completes the expansion: butil/macros.h defines the same macro without one and wins under #ifndef in TUs that happen to see butil first, so the two definitions must stay call-site compatible - format_v2/native/native_reader.h: include runtime/runtime_profile.h (RuntimeProfile::Counter members need the complete type; it rode in via pod_array -> thread_context) - bm25_similarity.cpp: drop the dead `using namespace inverted_index` (the only declaration of that namespace lives in exec_env.h and rode the same edge) - inverted_index_searcher.h, index_file_writer.h, ann_index_writer.h, query/query.h: wrap their bare <CLucene.h> includes in the -Wconversion suppression (inverted_index_common_impl.h pattern); whether CLucene's first expansion lands inside a suppressed region depends on include order Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> (cherry picked from commit c2bf234)
…nteger - core/pod_array.h no longer includes runtime/thread_context.h: dead include left over from the PODArray memory-tracking experiment (apache#50549); the tracking logic has since moved into Allocator and pod_array.h references no thread_context symbol. 203 TUs stop seeing thread_context.h and 195 of them stop seeing exec_env.h (differential payload 1.37MB per TU). - core/column/column.h no longer includes exec/sort/hybrid_sorter.h (core -> exec layering violation): HybridSorter only appears in virtual signatures, which the forward declaration covers; pdqsort/timsort leave 808 TUs. - wide_integer_impl.h defines the from-double members (set_multiplier / wide_integer_from_builtin(double)) inline only where the 80-bit long double exists (LDBL_MANT_DIG == 64 — unchanged and still constexpr there) or inside the dedicated impl TU core/wide_integer_from_double.cpp; every other TU sees declarations only and drops boost/multiprecision + boost/math from its closure (4.27MB preprocessed x 1169 TUs; the members were never constexpr on these platforms, so no constant evaluation is lost). - check-header-deps.py: two new rules (pod_array.h !-> thread_context.h, column.h !-> exec/sort/). No formal rule for the boost edge: the scanner is preprocessor-blind and would flag the impl TU's gated include; the macro structure is self-guarding (breaking it fails the impl TU build). Preprocessed closure of column.h: 16.72MB -> 10.15MB (-39%). Syntax sweep: 1358 TUs; failures identical to the 4 known pre-existing merge artifacts (stale gensrc x3, apache#66242 template-only call site), zero new. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> (cherry picked from commit 13fca3d)
…iers
Pure-additive / behavior-preserving preparation so that two kinds of
template-instantiation edges can be cut next: both parquet decoder.h
headers' util/rle_encoding.h include (their inline BaseDictDecoder bodies
force ~530 TUs to instantiate the RleBatchDecoder<uint32_t> ->
GetLiteralValues -> UnpackBatch -> UnpackValues chain at ~0.43 CPU s per
TU, 231 CPU s total) and the FMT_COMPILE formatter instantiations that
uint24.h / large_int_value.h push into ~1150 TUs (53.5 CPU s). No include
edge is removed here:
- format/parquet/decoder.h: move set_data, skip_values and the defaulted
~BaseDictDecoder out of line into decoder.cpp; everything moved is a
per-page/per-batch virtual call that is dispatched through the vtable at
every call site already, so outlining changes no generated call. With the
dtor out of line the unique_ptr<RleBatchDecoder<uint32_t>> member no
longer needs the complete type in the header; forward-declare it
- format_v2/parquet/reader/native/decoder.h: same treatment for set_data,
decode_dictionary_indices, decode_selected_dictionary_indices,
_decode_fragmented_selection, _decode_and_validate_skipped,
_decode_dictionary_values and the dtor; the pure orchestrator bodies
(skip_values, decode_dictionary_values, decode_selected_dictionary_values)
stay inline as they only call the now-declared members
- decoder.cpp (both trees): include util/rle_encoding.h and
common/cast_set.h directly (cast_set previously rode in through
rle_encoding.h)
- core/uint24.h: move to_string (the FMT_COMPILE("{:04d}-{:02d}-{:02d}")
date formatter, the single biggest fmt instantiation in the codebase)
into a new core/uint24.cpp; a std::string-returning formatter is
allocation-dominated, so the outlined call is noise
- core/value/large_int_value.h: move to_buffer/to_string x2 (the
FMT_COMPILE int128 formatters) into large_int_value.cpp, which now
includes fmt/compile.h + fmt/format.h directly instead of riding its own
header
The RleBatchDecoder methods stay fully inline in rle_encoding.h for the
real decode TUs; the per-value-hot LevelDecoder::get_next path is
deliberately untouched.
(cherry picked from commit df9ec8f)
The one-line deletions the previous commit prepared, plus the guard rules that lock them in: - format/parquet/decoder.h and format_v2/parquet/reader/native/decoder.h drop util/rle_encoding.h (RleBatchDecoder<uint32_t> is forward-declared; the complete type is only needed by decoder.cpp). New trace run 20260805_131321 measured the old edges at 531 TUs x 435 ms = 231 CPU s of RleBatchDecoder/BitPacking/BatchedBitReader instantiation, plus the 2038-line rle_encoding/bit_stream_utils/bit_packing family reparsed in each of those TUs. The real decode TUs keep including rle_encoding.h directly and inline the chain exactly as before - core/uint24.h drops <fmt/compile.h> and core/value/large_int_value.h drops <fmt/compile.h> + <fmt/format.h>: their formatter bodies moved to the matching .cpp files, which ends the FMT_COMPILE date/int128 formatter instantiation (53.5 CPU s over ~1150 TUs) in every includer - check-header-deps.py: two new rules pin both decoder.h headers away from util/rle_encoding.h (19 rules total, all passing); the fmt cuts are noted in a comment because the scanner only follows quoted project includes Full -fsyntax-only sweep: 1364/1364 TUs clean with the cuts applied (the single failure during arbitration was large_int_value.cpp riding its own header for fmt, fixed in the preparation commit). (cherry picked from commit 624d039)
The four cut-edge commits on this branch were validated with a -fsyntax-only sweep over be/src only, so the 1024 be/test TUs were never compiled against the slimmed graph. A full `ninja -k 0` run (2422 targets) surfaced 8 failing TUs in 4 families. Nothing here restores a cut edge, and no production header gains an include; the RLE fix in fact tightens the invariant decoder.h already claimed. - core/wide_integer_from_double.cpp: -Werror,-Wunused-macros. Where the 80-bit long double exists, wide_integer_impl.h takes its `#if (LDBL_MANT_DIG == 64)` branch and never evaluates the `#elif defined(...)` that reads DORIS_WIDE_INTEGER_FROM_DOUBLE_IMPL_TU, so the definition looked unused. The existing instantiation guard now reads the macro itself. `defined(...)` must stay the left operand -- the preprocessor short-circuits `&&`, so a false left term skips it and the warning returns. - format/parquet/decoder.h and format_v2/parquet/reader/native/decoder.h: outline BaseDictDecoder's default ctor next to the dtor that was already outlined. A defaulted-in-class ctor is defined in every TU that constructs a derived decoder and it odr-uses ~unique_ptr<RleBatchDecoder<uint32_t>>, so the header's own comment -- that the complete type is only needed in decoder.cpp -- did not hold. Both trees had the defect; only the format/ one had a test reaching it. - block_test / column_variant_v2_test: declare the RuntimeProfile and HybridSorter they use directly. They had been riding pod_array.h -> thread_context.h and column.h -> exec/sort/hybrid_sorter.h. - asof_join_test and the two fix_length_dict_decoder tests: include-order- sensitive, so the cost stays in the tests rather than in a production header. AsofIndexGroup::sort_and_finalize calls the global pdqsort, which ADL cannot reach from std::vector's iterators; FixLengthDictDecoder::_decode_values dereferences a unique_ptr<RleBatchDecoder<uint32_t>> that depends on no template parameter. Both bind where the template is parsed, not where it is instantiated, so the supplying include has to come first. The two decoder tests carry clang-format off/on because the formatter would otherwise sort the include back after the header under test and break the build again. Verified: `sh run-be-ut.sh` links test/doris_be_test with 0 failures; all 10 files pass clang-format --dry-run --Werror. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01V3BVeKrPS3jQ2bagcuGHBQ (cherry picked from commit f8b01ae)
Pure-additive / behavior-preserving preparation so that the hash-table
variant machinery (exec/common/{agg,join,set,distinct_agg}_utils.h) can be
cut out of exec/pipeline/dependency.h and rec_cte_shared_state.h next.
Those two headers hold non-template in-class inline bodies (SharedState
constructors/destructors/close paths, three std::visit dispatches) that
reference the full variant types, and such bodies are semantically
analyzed when the header is parsed: every one of the ~128 TUs that
transitively include dependency.h instantiates the whole
AggregatedDataVariants/JoinDataVariants/SetDataVariants surface at a flat
~0.85 CPU s per TU (~130 CPU s total), whether it uses them or not. No
include edge is removed here:
- dependency.h: move out of line into dependency.cpp everything whose body
touches a variant -- AggSharedState ctor/dtor/_close_with_serialized_key/
_close_without_key, BucketedAggSharedState PerInstanceData ctor, dtor,
_close and _close_one_agg_data, both HashJoinSharedState ctors, and a
new user-declared SetSharedState ctor/dtor pair that takes over the
make_unique<SetDataVariants>() default member initializer. All of it is
per-query setup/teardown, so outlining costs nothing at runtime
- dependency.h: forward-declare the variant structs, spell the members as
plain std::unique_ptr/std::vector<size_t> instead of the utils-provided
aliases, redeclare AggregateDataPtr, and directly include the light
dependencies that used to ride in through the utils chain (core/arena.h,
exprs/vexpr_fwd.h, util/stopwatch.hpp, cast_set/exception/
factory_creator, <list>/<queue>/<set>)
- exec/common/join_op_utils.h (new): JoinOpVariants, is_asof_join*, and
the AsofIndexGroup/AsofIndexVariant family move here out of
join_utils.h, which re-exports it; dependency.h holds these by value,
and this light header depends only on thrift enums, pdqsort and std
containers -- not on the hash tables
- rec_cte_shared_state.h: same treatment for the fourth amplifier; the
emplace_block std::visit over DistinctDataVariants plus the brpc-heavy
build_basic_param/send_data_to_targets bodies move to a new
rec_cte_shared_state.cpp (CMake GLOB picks it up), with user-declared
ctor/dtor so unique_ptr<DistinctDataVariants> works forward-declared
- seed direct includes ahead of the cut: agg_utils.h into the three agg
operator headers whose inline code walks method_variant, join_utils.h
into join_build_sink_operator.h, distinct_agg_utils.h into
rec_cte_source_operator.cpp, and the utils/template_helpers includes
into dependency.cpp
Verified with a full -fsyntax-only sweep: 0 failing TU(s) of 1365.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
(cherry picked from commit e39c032)
…ncy.h With the SharedState bodies out of line (previous commit), drop the edges that made every pipeline TU pay for the hash-table variant machinery and two dead payload includes. Measured on the P3.5 research traces: a flat ~0.85 CPU s of parse-time template instantiation per TU across the ~128 TUs reached by dependency.h (105 of which never touch a variant), plus 1.36 MB/TU of preprocessed brpc/query_context payload riding the dead brpc_closure.h edge into ~100 TUs. Edges cut from exec/pipeline/dependency.h: - exec/common/agg_utils.h, set_utils.h: forward declarations suffice now - exec/common/join_utils.h -> exec/common/join_op_utils.h: the by-value JoinOpVariants/AsofIndexVariant members need complete types, so keep only the light join-op header split out in the previous commit - exec/operator/join/process_hash_table_probe.h: dead include (no symbol of it is referenced here); the probe machinery belongs to the join TUs - util/brpc_closure.h: dead include; it was the only route that carried runtime/query_context.h, runtime/thread_context.h and service/brpc.h into ~100 pipeline TUs - <concurrentqueue.h>: dead include (152 KB third-party single header); the moodycamel users include it themselves Also make BucketedAggSharedState::init_instances a non-template taking std::function (defined in dependency.cpp): non-dependent constructs in a member-template body are checked at definition time, so the old inline template still forced the destructor of unique_ptr<BucketedAggDataVariants> on every includer despite never being called there. Once-per-query cold path. exec/pipeline/rec_cte_shared_state.h (fourth amplifier, same recipe): - cut exec/common/distinct_agg_utils.h (DistinctDataVariants is only touched by rec_cte_shared_state.cpp now) and util/brpc_client_cache.h (the rpc send bodies live in the .cpp) Seeds arbitrated by two full -fsyntax-only sweeps (16 true positives): - process_hash_table_probe_impl.h includes join_utils.h (the INSTANTIATION_FOR macro names SerializedHashTableContext and friends, fixing all 12 *_join_impl.cpp TUs); process_hash_table_probe.h becomes self-contained (runtime_profile.h for its Counter members) - hashjoin_probe_operator.h includes process_hash_table_probe.h (the by-value std::variant<ProcessHashTableProbe<...>> needs the definition) - direct utils/hash_map_util includes for the TUs that dereference the variants: hashjoin_probe_operator.cpp, hashjoin_build_sink.cpp, the three set operator cpps, the three aggregation source cpps - materialization_opertor.h includes service/brpc.h for its brpc::Controller member; local_exchanger.h includes concurrentqueue.h for its ConcurrentQueue members Guards: check-header-deps.py gains five rules (24 total, all passing): dependency.h and rec_cte_shared_state.h must not reach exec/common/hash_table/ (phmap_fwd_decl.h excepted), dependency.h must not regain process_hash_table_probe.h / brpc_closure.h, and rec_cte_shared_state.h must not regain brpc_client_cache.h; the concurrentqueue.h ban is recorded as a comment (third-party angle include, outside the scanner's reach). Verified: full -fsyntax-only sweep 0 failing of 1365; header layering 24/24. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> (cherry picked from commit 82d8ce7)
The PCH already pulls in wide_integer_impl.h (via storage/olap_common.h) in declaration-only mode, so its pre-spent include guard would leave the from-double explicit instantiations in this TU without definitions. Skip the PCH for this single TU so DORIS_WIDE_INTEGER_FROM_DOUBLE_IMPL_TU takes effect. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> (cherry picked from commit ae1f7b1)
`std::min(*rows, ROWS - _state->current_ordinal)` fails template argument deduction wherever size_t and ordinal_t are distinct types: on macOS/arm64 size_t is `unsigned long` while uint64_t -- and hence ordinal_t -- is `unsigned long long`, so _Tp is deduced conflictingly and none of the four std::min overloads match. On Linux the two spell the same type, which is why CI never saw it. Name the template argument explicitly at the three call sites. Both types are 64-bit unsigned, so nothing is narrowed. The break arrived with apache#66204 and has nothing to do with the include-edge cuts on this branch, but it stops be/test from building on macOS at all -- which is where this branch is verified -- so it is fixed here. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Gdfkk7RqgD5e3Uv7bTM3NV
morningman
requested review from
Gabriel39,
airborne12,
csun5285,
eldenmoon,
gavinchou and
yiguolei
as code owners
August 11, 2026 16:21
Contributor
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
Contributor
Author
|
run buildall |
1 similar comment
Contributor
Author
|
run buildall |
Contributor
Author
|
run buildall |
…ply it
`column.h -> exec/sort/hybrid_sorter.h` was carrying `gfx/timsort.hpp`, whose
line 37 is `#include <ranges>`. Four files consumed `<ranges>` through that
chain without ever asking for it, and the previous commit's cut takes it away.
Why the local build did not see this: it runs with `ENABLE_PCH=ON`, and
`cmake_pch.hxx` supplies `<ranges>` to every TU regardless of that TU's own
includes, so the debt stays invisible. The upstream compile lane builds without
a PCH, where nothing covers for it. Compiling the same file on macOS with the
PCH genuinely stripped reproduces the identical error, so this is not a
libc++/libstdc++ difference -- it is the PCH masking a missing include, exactly
the failure mode `syntax_sweep.py --no-pch` exists to expose.
- storage/tablet/base_tablet.cpp: `std::views::transform` / `std::views::filter`
-- this is the one the CI compile lane caught.
- cloud/cloud_meta_mgr.{h,cpp}: three declarations and three definitions
constrained on `std::ranges::range auto&&`. The Cloud target is compiled
after Storage, so the build stopped before reaching it; it would have failed
next.
- storage/compaction/compaction.cpp: `std::ranges::reverse_view`. This one still
compiles, because simdjson.h happens to remain in its closure and also pulls
`<ranges>` -- included here so it stops depending on that accident.
Found by diffing each consumer's include closure before and after the cut and
matching it against the symbols the removed providers supply, rather than by
waiting for another CI round trip.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Gdfkk7RqgD5e3Uv7bTM3NV
morningman
force-pushed
the
be-build-opt-1-include-cuts
branch
from
August 12, 2026 00:22
06351a4 to
0312902
Compare
Contributor
Author
|
run buildall |
…::agg_data
GCC rejects the forward-declared `unique_ptr<DistinctDataVariants>` member that
the previous commits introduced, in every TU that merely includes
rec_cte_shared_state.h:
unique_ptr.h:91:23: error: invalid application of 'sizeof' to incomplete
type 'doris::DistinctDataVariants'
required from 'std::unique_ptr<_Tp, _Dp>::~unique_ptr()'
rec_cte_shared_state.h:42:54: required from here
Column 54 is the `nullptr` of `agg_data = nullptr`. A default member
initializer is part of the class definition, so GCC instantiates the member's
destructor while completing the class -- which needs `DistinctDataVariants` to
be complete -- even though the constructor and destructor of RecCTESharedState
are both defined out of line in rec_cte_shared_state.cpp, where it is complete.
Clang defers that instantiation to the point the constructor is actually
defined, which is why the clang lanes and the macOS build were green while the
gcc lane failed on rec_cte_anchor_sink_operator.cpp and rec_cte_sink_operator.cpp.
`= nullptr` was redundant to begin with: `unique_ptr`'s default constructor
already leaves it null, and `RecCTESharedState::RecCTESharedState() = default;`
in the .cpp performs that initialization where the type is complete. So dropping
the initializer changes no behavior and removes the only instantiation point
that required completeness in the header.
Swept the rest of the tree for the same shape: the only other
`unique_ptr<DistinctDataVariants>` with an initializer is in
distinct_streaming_aggregation_operator.h, which includes distinct_agg_utils.h
directly and therefore has the complete type.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Gdfkk7RqgD5e3Uv7bTM3NV
Contributor
Author
|
run buildall |
1 similar comment
Contributor
Author
|
run buildall |
Contributor
TPC-H: Total hot run time: 28728 ms |
Contributor
TPC-DS: Total hot run time: 158781 ms |
Contributor
ClickBench: Total hot run time: 24.11 s |
gavinchou
approved these changes
Aug 12, 2026
Contributor
|
PR approved by at least one committer and no changes requested. |
Contributor
|
PR approved by anyone and no changes requested. |
Contributor
Author
|
run vault_p0 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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.pyrules.Wave 1 — three hot edges (
Prepare cutting three hot edges+Cut three hot include edges)core/pod_array.h→runtime/thread_context.hAllocatorandpod_array.hnames nothread_contextsymbolthread_context.hand 195 of them stop seeingexec_env.h(1.37 MB/TU differential payload)core/column/column.h→exec/sort/hybrid_sorter.hcore→execlayering violation reaching 808 TUsHybridSorteronly appears in virtual signatures, so a forward declaration covers it; theBE_TEST-onlyget_permutation_defaultbody (it constructs aHybridSorterby value) moves tocolumn.cppcore/wide_integer_impl.h→boost/multiprecision(unconditional)constexpr) only whereLDBL_MANT_DIG == 64; elsewhere they compile once in a newcore/wide_integer_from_double.cppwith explicit instantiations forinteger<128|256, signed|unsigned>. They were neverconstexpron those platforms, so no constant evaluation is lostPreprocessed closure of
column.h: 16.72 MB → 10.15 MB (-39%).Wave 2 — two instantiation amplifiers: RLE and FMT_COMPILE
Both parquet
decoder.htrees includedutil/rle_encoding.honly because inlineBaseDictDecoderbodies made ~530 TUs instantiate theRleBatchDecoder<uint32_t>→GetLiteralValues→UnpackBatch→UnpackValueschain — 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 indecoder.cpp. Everything moved is dispatched through the vtable at every callsite already, so no generated call changes; the real decode TUs keep including
rle_encoding.hdirectly and inline the chain exactly as before, and theper-value-hot
LevelDecoder::get_nextpath is deliberately untouched.core/uint24.handcore/value/large_int_value.hpushed theirFMT_COMPILEformatter instantiations (the
"{:04d}-{:02d}-{:02d}"date formatter is thesingle biggest fmt instantiation in the tree, plus the int128 formatters) into
~1150 TUs, 53.5 CPU s. Those bodies move to
.cppfiles and both headersdrop
<fmt/compile.h>/<fmt/format.h>. They returnstd::stringand areallocation-dominated, so the now-outlined call is noise.
Wave 3 — the DataVariants amplifier behind
dependency.hexec/pipeline/dependency.handexec/pipeline/rec_cte_shared_state.hholdnon-template in-class inline bodies — SharedState constructors, destructors,
close paths and three
std::visitdispatches — that name the fullAggregatedDataVariants/JoinDataVariants/SetDataVariants/DistinctDataVariantssurface. Such bodies are semantically analyzed when theheader is parsed, not when they are called, so every one of the ~128 TUs that
transitively include
dependency.hinstantiated 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.cppand a newrec_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 lightexec/common/join_op_utils.h(JoinOpVariants and theAsofIndexGroupfamily split out ofjoin_utils.h, which re-exports them;dependency.hholds 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 carryingquery_context.h,thread_context.handservice/brpc.hinto ~100 pipeline TUs (1.36 MB/TU);<concurrentqueue.h>— dead include (152 KB third-party single header);util/brpc_client_cache.hfromrec_cte_shared_state.h— the rpc send bodies live in the.cppnow.BucketedAggSharedState::init_instancesalso becomes a non-template takingstd::function: non-dependent constructs in a member-template body are checkedat definition time, so the old inline template forced the destructor of
unique_ptr<BucketedAggDataVariants>on every includer despite never beingcalled there.
One CI-critical commit rides along
Opt wide_integer_from_double.cpp out of the PCHmust ship in this PR, not asa follow-up. Upstream's clang toolchain defaults to
ENABLE_PCH=ON(
be/CMakeLists.txt), andpch.htransitively includeswide_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.
Head-to-head on exactly this PR's content
Cold, cache-free BE builds of this PR's merge base (
c29075a7e10) and its head(
03129023ee5), run back-to-back on the same machine with the repo's own--compile-benchharness — dedicated always-cold build dirs, ccache disabled,-j10,ENABLE_PCH=ON, and both runs started from the same cooled state(load1 2.1 vs 2.3) so neither side pays for the other's heat.
The improvements land exactly on the predicted targets —
pipeline_fragment_context.cpp-6.0 s,
rec_cte_anchor_sink_operator.cpp-5.0 s,operator.cpp-3.7 s,data_queue.cpp-3.0 s (all W3dependency.hconsumers), andbyte_array_dict_decoder.cpp-2.5 s (the W2 decoder edge).Of the 12 regressions,
dependency.cpp+2.6 s is by design: that is the TU theSharedState bodies were moved into, so it pays once for what ~128 TUs stop paying.
The rest sit in the noise floor of a
-j10run, and there is direct evidence forthat floor:
gensrc/build/gen_cpp/cloud.pb.cc, a generated protobuf TU this PRcannot touch, moved +2.3 s between the two runs.
Why no end-to-end total-wall number is quoted: the two trees differed outside
the build phase — the baseline tree skipped the contrib submodule step while the PR
tree re-fetched it (+58.1 s), and its gensrc was already generated (-5.6 s). Those
phases are not compilation, so only the build-phase and Σ-TU figures above are
attributable to the change.
One caveat worth stating: these numbers are with
ENABLE_PCH=ON. A PCH alreadyamortizes exactly the kind of shared headers this PR is cutting, so it masks part of
the win — and the upstream compile lane builds without a PCH (its command line
carries no
-include-pch). The effect there should be larger, not smaller.Per-wave numbers from the development branch
be/srcCPU -3.6%These were taken on the batch branch at
-j5/-j6against an older base, so theydo not add up to the head-to-head figure above; they are included because they
attribute the win to each wave separately.
Risk and verification
-fsyntax-onlysweep 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/srconly, so a fullninja -k 0over 2422 targets was run to reach the 1024be/testTUs; it surfaced 8 failing TUs in 4 families, all repaired inRepair 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 indecoder.cppdid not hold — in both trees, though only theformat/one had a test reaching it.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_belinks.compile_commands.jsonconfirmswide_integer_from_double.cppis the one first-party TU compiled without the PCH.syntax_sweep.py --no-pch, the gate added in [opt](build) Add build-timing and header-closure sweep tooling #66616): 0 failing TUs of 1380. This is the gate that matters for a PR that cuts include edges — a normal build withENABLE_PCH=ONcannot see a missing include that the PCH happens to supply, which is precisely how the<ranges>breakage in the last commit reached CI before it reached me.check-header-deps.pygains rules pinningpod_array.h !-> thread_context.h,column.h !-> exec/sort/, and bothdecoder.hheaders!-> util/rle_encoding.h(19 rules total, all passing).Proactive disclosure
Make hierarchical_data_iterator_test compile on macOS arm64.std::min(*rows, ROWS - current_ordinal)cannot deduce_Tpwheresize_tisunsigned longandordinal_t(uint64_t) isunsigned long long, which is the case on macOS/arm64 but not on Linux — so CI is green whilebe/testdoes not build on macOS at all. It arrived with [feature](variant) Support ColumnVariantV2 segment reads and writes #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.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#includeto 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.check-header-deps.pyis 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.clang-format off/onaround their includes.asof_join_testand the twofix_length_dict_decodertests are include-order-sensitive: the supplying include has to come before the header under test (ADL cannot reach the globalpdqsortfromstd::vector's iterators; aunique_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_ASSIGNinstorage/olap_define.hloses its trailing semicolon.butil/macros.hdefines the same macro without one and wins under#ifndefin 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
-fsyntax-onlysweeps above)Behavior changed:
Does this need documentation?