[opt](build) 2/4: Speed up BE full build a further ~8% by cutting instantiation amplifiers and three more hot edges - #66510
Open
morningman wants to merge 9 commits into
Open
[opt](build) 2/4: Speed up BE full build a further ~8% by cutting instantiation amplifiers and three more hot edges#66510morningman wants to merge 9 commits into
morningman wants to merge 9 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>
…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>
…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.
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).
morningman
requested review from
Gabriel39,
airborne12,
csun5285,
eldenmoon,
gavinchou and
yiguolei
as code owners
August 5, 2026 16:02
Contributor
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
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
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>
…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>
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>
Contributor
Author
|
run buildall |
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?
Issue Number: close #xxx
Related PR: #66400
Problem Summary:
Part 2 of the BE build-speed series started in #66400. That PR cut the
high-payload include edges out of the
exec_env.h/runtime_state.h/thread_context.hsuperhighways; this one finishes the include-edge line andthen goes after a cost the include audit cannot see: template-instantiation
amplifiers — small headers whose inline bodies force hundreds of TUs to
instantiate expensive template chains they never use.
Measured with the
--compile-benchharness from #66400 (cold, cache-free,-j5, Apple M5 Pro, clang 20), same code base A/B:This PR: 30m48s -> 28m19s (-8.1%). Cumulative with #66400: 38m54s -> 28m19s
build wall (-27%).
Wave A — three more hot include edges (commits 1+2, prepare/cut):
core/pod_array.h -> runtime/thread_context.h: dead include left over fromthe PODArray memory-tracking experiment ([opt](memtracker) Optimize
PODArraymemory tracking accuracy #50549); dragged thread_context /exec_env into 203 TUs of core/
core/column/column.h -> exec/sort/hybrid_sorter.h: core->exec layeringviolation reaching 808 TUs; HybridSorter is only named in virtual signatures
(now forward-declared, the BE_TEST-only body sunk into column.cpp)
core/wide_integer_impl.h -> boost/multiprecision/cpp_bin_float.hpp:4.27 MB of preprocessed closure in 1169 TUs, needed only for the
from-double conversion on platforms without an 80-bit long double; now
compiled once in a dedicated TU (
core/wide_integer_from_double.cpp) withexplicit instantiations, x86 semantics untouched.
column.hclosure:16.72 MB -> 10.15 MB (-39%)
Wave B — instantiation amplifiers (commits 3+4, prepare/cut):
decoder.hheaders definedBaseDictDecoder's virtualmethods inline, and those bodies construct/call
RleBatchDecoder<uint32_t>.A call inside an inline body implicitly instantiates the referenced
templates in every TU that includes the header, so 531 TUs (cloud/,
information_schema/, python UDF... most of them nowhere near parquet) each
spent ~435 ms instantiating the GetBatch -> GetLiteralValues -> UnpackBatch
-> UnpackValues chain: 231 CPU s total, plus the 2038-line
rle_encoding/bit_stream_utils/bit_packing family reparsed in each.
Everything moved out of line is a per-page/per-batch virtual call that
is already dispatched through the vtable at every call site, so outlining
changes no generated call. The real decode TUs keep including
rle_encoding.h directly and inline the chain exactly as before; the
per-value-hot
LevelDecoder::get_nextis deliberately untouched.core/uint24.h::to_stringheld the single biggest fmt instantiation in thecodebase (
FMT_COMPILE("{:04d}-{:02d}-{:02d}"), 53.5 CPU s over 1147 TUs);it and
LargeIntValue's int128 formatters moved into .cpp files keepingFMT_COMPILE — the runtime formatting path is instruction-identical, only
the allocation-dominated
std::stringbuilders lose cross-TU inlining.Why extern template was rejected for the RLE chain: the methods are defined
in-class, hence implicitly inline, and explicit instantiation declarations do
not suppress implicit instantiation of inline functions ([temp.explicit]);
moving them out-of-class to make it work would de-inline the hot decode loops
in the real decode TUs. Outlining the cold virtual callers is strictly better.
Guards: 4 new rules in
build-support/check-header-deps.py(19 total, allpassing) pin
pod_array.h,column.hand bothdecoder.hheaders away fromthe cut subtrees; the fmt cuts are recorded as a comment (the scanner only
follows quoted project includes).
Release note
None
Check List (For Author)
Test
build-support/compile-bench/syntax_sweep.py: 1364/1364 be/src TUspass
-fsyntax-onlyafter every cut (re-run on the rebased treeagainst current master)
build-support/check-header-deps.py: 19/19 layering rules passsh build.sh --compile-benchcold builds succeed after eachwave;
report.py compareshows 483 files faster / regressionsconfined to the scheduling-sensitive 4 GB tail TUs whose include
closure is untouched by this PR (machine noise, same signature as
documented in [opt](build) 1/4: Speed up BE full build ~22% by cutting hot-header include edges #66400)
Behavior changed:
Does this need documentation?
Check List (For Reviewer who merge this PR)
🤖 Generated with Claude Code