[opt](build) Declare extern template for existing explicit instantiations - #66807
Open
morningman wants to merge 6 commits into
Open
[opt](build) Declare extern template for existing explicit instantiations#66807morningman wants to merge 6 commits into
morningman wants to merge 6 commits into
Conversation
column_vector.cpp, column_decimal.cpp and column_string.cpp already instantiate their class templates explicitly (18 + 5 + 2 types), but without extern declarations in the headers every including TU still instantiates all reachable members implicitly, only for the linker to deduplicate them again: multiply.o alone carried 629 weak definitions that are provided by the explicit instantiations. Declaring them extern suppresses the per-TU work across the ~380 TUs including these headers. Measured on the sentinel TUs (no-PCH, single compile): multiply 78.7s -> 77.3s with .text -5%, plus 56.3s -> 53.7s with .text -9%, 629 weak defs gone from each; the linked doris_be shrinks by 36 MB (-1.9%). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0121aRtZYjjYdNr2a6z8BLzR
data_type_number_base.cpp, data_type_decimal.cpp, data_type_number_serde.cpp, data_type_decimal_serde.cpp, data_type_string_serde.cpp and data_type_date_or_datetime_serde.cpp already instantiate their class templates explicitly (16 + 5 + 16 + 5 + 3 + 2 types); mirror them with extern template declarations in the headers so including TUs no longer re-instantiate the reachable members. These headers propagate through DataTypeNumber/Block into essentially every TU, same surface as P1-1. data_type_decimal_serde.h forward-declared ColumnDecimal but its member layout uses ColumnDecimal<T>::Container, so the explicit instantiation declaration needs the complete type: replace the forward declaration with the real include (itself extern'd since P1-1). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0121aRtZYjjYdNr2a6z8BLzR
…ions operator.cpp, scan_operator.cpp, join_probe_operator.cpp, join_build_sink_operator.cpp, the set operator family, partitioner.cpp and analytic_sink_operator.cpp already instantiate their operator class templates explicitly (99 specializations in total); mirror them with extern template declarations so consumer TUs (pipeline construction, factories, every operator TU including a sibling header) stop re-instantiating the operator base members. Placement follows argument visibility: the PipelineX(Sink)LocalState block sits in operator.h where dependency.h makes every SharedState complete, except the RecCTE pair whose shared state lives in rec_cte_shared_state.h, so those go to the rec_cte operator headers. Specializations parameterized by per-operator LocalState/Writer types go to the header defining that type. BE_TEST-only instantiations keep the same guard. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0121aRtZYjjYdNr2a6z8BLzR
…tions Mirror the remaining explicit instantiation definitions with extern template declarations: frame-of-reference codec (22), inverted index column writer (24), both parquet reader families (12 + 12, distinct namespaces), query_v2 scorers (11), DateV2Value (2), json_each table function (2), JSONDataParser and the page-cache footer page (1 + 1). Two support changes surfaced by the extern declarations: - ForEncoder<uint24_t>::numeric_limits_max is specialized in the .cpp, so the header must declare the specialization before the extern block (specialization-after-instantiation otherwise). - disjunction_scorer.h / occur_boolean_weight.h consumed the combiner alias types without including score_combiner.h. Skipped, cost exceeds the measured duplication: InvertedIndexVisitor (member specializations matches/compare would all need header declarations, 83 weak defs repo-wide), ResultBlockBuffer (class instantiation needs the complete ctx types from exec/sink, a layering violation for 36 weak defs), VerticalBetaRowsetWriter (derives from its argument; cloud/binlog writer types are not visible in the header, 54 weak defs). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0121aRtZYjjYdNr2a6z8BLzR
DataTypeNumberBase is explicitly instantiated but the derived DataTypeNumber (the class every factory and arithmetic TU actually names via the DataTypeInt*/Float*/Bool aliases) was not, so each such TU re-emitted its vtable, typeinfo and equals for every used type. Instantiate the eight aliased specializations next to the base ones in data_type_number_base.cpp and declare them extern in the header. Surveyed for the same gap, measured as not worth covering: PODArray (members mostly inline away, <= 7 weak defs per TU) and the COW/COWHelper bases (already suppressed transitively by the P1-1 column externs; only ~24 trivial nested-pointer symbols remain in multiply). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0121aRtZYjjYdNr2a6z8BLzR
The extern template declarations force class-level instantiation of InvertedIndexColumnWriter, whose CppType member alias needs the CppTypeTraits specializations from storage/types.h. The header only forward-declared the primary template, so TUs that did not pull the traits in transitively (storage unity_16) failed to compile; the defining TU passed only via transitive includes. Include the real header and drop the forward declaration. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0121aRtZYjjYdNr2a6z8BLzR
morningman
requested review from
Gabriel39,
airborne12,
csun5285,
eldenmoon,
gavinchou,
liaoxin01 and
yiguolei
as code owners
August 16, 2026 13:14
Contributor
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
Contributor
Author
|
run buildall |
11 tasks
Contributor
TPC-H: Total hot run time: 17457 ms |
Contributor
TPC-DS: Total hot run time: 83455 ms |
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
2 participants
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: #66510, #66789
Problem Summary:
be/srcalready contains ~221template classexplicit instantiationdefinitions (columns, DataTypes, SerDes, the operator families, …). But almost
none of them are announced in the corresponding headers with an
extern templatedeclaration. The result: every consumer TU that touchesColumnVector<T>/DataTypeDecimalSerDe<T>/AsyncWriterSink<W, P>stillimplicitly instantiates the whole class again, compiles the member functions
as weak symbols, and the linker then throws all the duplicates away. The
explicit-instantiation TU does the same work one more time. We pay the template
instantiation cost N+1 times and keep exactly one copy.
extern templateis the standard C++11 tool for this: it suppresses implicitinstantiation in consumers and pins code generation to the one TU that already
carries the explicit definition. It changes symbol ownership only, not
generated code — a consumer TU compiled before/after this PR produces
byte-identical code for its own functions (verified on a control TU during the
original measurement round).
What the commits do:
column_vector.h18,column_decimal.h5,column_string.h2): declare extern the 25 instantiations defined in thematching
.cppfiles.date/datetime/decimal/number/string SerDe instantiation sets.
AsyncWriterSink,DataSinkOperatorX,OperatorX, partitioners, aggregation/table-functionoperators — all instantiated centrally (mostly in
operator.cpp) since theoperator refactor, never externed.
iterators, frame-of-reference coding, phrase queries, JSON parser.
DataTypeNumber<T>: the base class was explicitly instantiated but thederived class itself was not — instantiations existed nowhere, so every user
built the full class. Adds the 8 explicit definitions in
data_type_number_base.cppplus matching externs.inverted_index_writer.hforward-declared
CppTypeTraitsitself; triggering class-level instantiationfrom the extern requires the real definition, and exactly one storage unity
batch (of 13k+ TUs) lacked it transitively. Include
storage/types.hdirectly.
Measured results
Numbers below were taken on the original development branch before the unity
line landed (macOS arm64, clang 20,
-j6, no PCH for the sentinel probes),because that is where the mechanism was isolated. Landing after unity
(#66789), part of the win is already absorbed — sibling files inside one unity
batch share a single implicit instantiation — so the remaining surface here is
the SKIP-listed heavy individual TUs, cross-batch dedup, and the BE UT tree:
-j6)multiply.cppsentinel TU (no PCH)plus.cppsentinel TU (no PCH)doris_besize (pre-unity layout)Validation of this PR's tree (master + these 6 commits, macOS arm64 clang20,
unity=ON + PCH=ON):
doris_belinks at 319MB(same as current master);
extern templatedeclarations added hereresolve to an existing explicit instantiation definition in the current tree;
BUILD_TYPE_UT=Debug): 8501/8501 edges compiled,doris_be_testlinks with zero duplicate/undefined symbols — the sensitive surface for
an extern-template change, since tests link the full static-library set;
git clang-formatclean against master.Methodology, and what we deliberately did NOT extern
The go/no-go gauge for each candidate was weak symbols owned by consumer
.ofiles (llvm-nm -C | grep ' [VvWw] 'filtered by the class prefix), notthe count of instantiation statements. By that gauge three whole families were
rejected as free-of-benefit and are intentionally absent here:
Allocator(48 instantiation sites): consumers own ≈0 weak symbols of it;PODArray: same;COWHelperbase-class instantiations: same.Three further individual candidates were dropped because their include
topology would have needed real surgery for a mechanism whose gain there is ≈0.
Risks / disclosures
extern templatemoves symbolownership; it does not change what code is generated for the anchor TU, and
inline/constexpr members remain inlinable at call sites exactly as before.
extern template+ in-class-defined members slightly differently indiagnostics; the Performance pipeline (the only gcc lane) is the
authoritative check. Please watch its first round.
DORIS_DEV_DEBUG_INFOdeveloper knob that rode along in the originalbranch is intentionally not in this PR (unrelated mechanism, will be
proposed separately).