morningman opened a new pull request, #66807:
URL: https://github.com/apache/doris/pull/66807

   > Part of the BE build-time optimization series tracked in #66715.
   >
   > Split out of **https://github.com/apache/doris/pull/66510**. With the 
include-edge
   > surgery (#66400, #66672) and the unity-build line (#66712, #66776, #66789) 
merged,
   > this PR opens the third mechanism of the batch: making the **explicit 
template
   > instantiations we already have** actually pay for themselves by declaring 
them
   > `extern template` in the headers.
   
   ### What problem does this PR solve?
   
   Related PR: #66510, #66789
   
   Problem Summary:
   
   `be/src` already contains ~221 `template class` **explicit instantiation
   definitions** (columns, DataTypes, SerDes, the operator families, …). But 
almost
   none of them are announced in the corresponding headers with an
   `extern template` declaration. The result: every consumer TU that touches
   `ColumnVector<T>` / `DataTypeDecimalSerDe<T>` / `AsyncWriterSink<W, P>` still
   **implicitly 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 template` is the standard C++11 tool for this: it suppresses implicit
   instantiation 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:
   
   1. **Column classes** (`column_vector.h` 18, `column_decimal.h` 5,
      `column_string.h` 2): declare extern the 25 instantiations defined in the
      matching `.cpp` files.
   2. **DataType / SerDe surface** (6 headers): 47 externs for the
      date/datetime/decimal/number/string SerDe instantiation sets.
   3. **Exec operator families** (39 headers, 99 externs): `AsyncWriterSink`,
      `DataSinkOperatorX`, `OperatorX`, partitioners, aggregation/table-function
      operators — all instantiated centrally (mostly in `operator.cpp`) since 
the
      operator refactor, never externed.
   4. **Narrow surfaces** (17 headers, 75 externs): parquet/orc readers, segment
      iterators, frame-of-reference coding, phrase queries, JSON parser.
   5. **`DataTypeNumber<T>`**: the base class was explicitly instantiated but 
the
      derived class itself was not — instantiations existed nowhere, so every 
user
      built the full class. Adds the 8 explicit definitions in
      `data_type_number_base.cpp` plus matching externs.
   6. **One-line correctness fix** the externs exposed: 
`inverted_index_writer.h`
      forward-declared `CppTypeTraits` itself; triggering class-level 
instantiation
      from the extern requires the real definition, and exactly one storage 
unity
      batch (of 13k+ TUs) lacked it transitively. Include `storage/types.h`
      directly.
   
   ### 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:
   
   | metric | before | after |
   |---|---|---|
   | full cold build (`-j6`) | 33m15s | **32m36s (−2.0%)** |
   | `multiply.cpp` sentinel TU (no PCH) | 78.7s | **73.0s (−7.3%)** |
   | `plus.cpp` sentinel TU (no PCH) | 56.3s | **52.0s (−7.6%)** |
   | weak-symbol overlap multiply∩plus | 1552 | **491 (−68%)** |
   | `doris_be` size (pre-unity layout) | 1856MB | **1809MB (−2.5%)** |
   
   Validation of this PR's tree (master + these 6 commits, macOS arm64 clang20,
   unity=ON + PCH=ON):
   
   - full BE build: 7446/7446 edges, **zero failures**, `doris_be` links at 
319MB
     (same as current master);
   - static pairing audit: all **266 `extern template` declarations** added here
     resolve to an existing explicit instantiation definition in the current 
tree;
   - BE UT (`BUILD_TYPE_UT=Debug`): 8501/8501 edges compiled, `doris_be_test`
     links with **zero duplicate/undefined symbols** — the sensitive surface for
     an extern-template change, since tests link the full static-library set;
   - `git clang-format` clean against master.
   
   ### Methodology, and what we deliberately did NOT extern
   
   The go/no-go gauge for each candidate was **weak symbols owned by consumer
   `.o` files** (`llvm-nm -C | grep ' [VvWw] '` filtered by the class prefix), 
not
   the 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;
   - `COWHelper` base-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
   
   - **Runtime impact: none by construction.** `extern template` moves symbol
     ownership; it does not change what code is generated for the anchor TU, and
     inline/constexpr members remain inlinable at call sites exactly as before.
   - **Cross-platform**: all local validation is macOS/clang20. gcc handles
     `extern template` + in-class-defined members slightly differently in
     diagnostics; the Performance pipeline (the only gcc lane) is the
     authoritative check. Please watch its first round.
   - The `DORIS_DEV_DEBUG_INFO` developer knob that rode along in the original
     branch is intentionally **not** in this PR (unrelated mechanism, will be
     proposed separately).
   


-- 
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]

Reply via email to