[opt](build) Enable unity builds for the Exec and Exprs targets - #66776
Merged
morningman merged 4 commits intoAug 14, 2026
Conversation
Prepares the Exec target for unity builds; no behavior change: - file_scanner.cpp / file_scanner_v2.cpp both defined the Iceberg delete content codes and is_iceberg_position_deletes_sys_table() in anonymous namespaces; move the shared trio into iceberg_scan_semantics.h. file_scanner_v2_test.cpp carried a third copy of the content codes (apache#66615 kept it file-local because this header move had not landed yet); it now uses the shared header too. - vtablet_writer.cpp / vtablet_writer_v2.cpp both defined a file-scope CLOSE_WAIT_EVENT_FALLBACK_MS; scope it into IndexChannel and VTabletWriterV2 respectively, and rename v2's static on_partitions_created() trampoline to on_partitions_created_v2 (the two file-scope functions cast to different writer types). - exchange_sink_operator.cpp's namespace-scope timer_name shadowed the function-local timer_name of unity siblings (-Wshadow -Werror); rename it to wait_for_dependency_timer_name. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ND7L1ZVTJf91TBpLwYSqct
Contributor
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
Contributor
Author
|
run buildall |
11 tasks
Fourth target of the unity rollout (after InformationSchema, the http glue and storage/index in apache#66712). 167 of 174 Exec TUs join unity batches of <=12 sources (14 unity TUs), gated on the ENABLE_UNITY_BUILD knob so -DENABLE_UNITY_BUILD=OFF still compiles every TU individually. Opted out: five files whose file-scope macros must not leak into unity siblings, plus the two heaviest template-instantiation TUs (operator.cpp, hashjoin_build_sink.cpp) which would dominate any batch they join; scan_operator.cpp is both. Measured on the validation build (-j14 + PCH): the 14 unity TUs take 425s of slot time under full parallel load where their members summed to ~1418s in the cold bench (3.3x); the whole target drops from 1533s to 539s. The largest unity TU compiles in 32s / 2.6GB RSS standalone, below the existing per-TU peaks elsewhere in the build. The archive keeps all external symbols (three weak linkonce_odr template instantiations dedup away) and shrinks 2450MB -> 641MB. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ND7L1ZVTJf91TBpLwYSqct
…, subobject linkage Prepares the Exprs target for unity builds; no behavior change: - dictionary_factory.h had no include guard at all, so any TU reaching it through two include paths fails with a class redefinition; add #pragma once. Under unity this also poisoned the whole batch: clang error recovery then produced spurious -Warray-bounds diagnostics in the unrelated function_encryption.cpp members of the same batch. - function_dict_get_many.cpp copied the DictGetState struct definition from function_dict_get.cpp at namespace scope; rename it to DictGetManyState so the two adjacent TUs can share a unity batch. - function_variant_element_v2.cpp kept OwnedPathSegment in an anonymous namespace while using it as a field of the externally-visible ResolvedVariantElementV2Path::Impl. gcc's -Wsubobject-linkage (-Werror) rejects exactly that as soon as the file is #included into a unity batch instead of being the main file of its TU; clang has no such warning, so only the gcc CI lane sees it. Move the struct to namespace scope (the name is unique to this TU). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ND7L1ZVTJf91TBpLwYSqct
Fifth target of the unity rollout (after InformationSchema, http, storage/index and Exec). 246 of 287 Exprs TUs join unity batches of <=8 sources (31 unity TUs), driven by DORIS_UNITY_BUILD so -DENABLE_UNITY_BUILD=OFF still compiles every TU individually. Opted out and kept individual: - the flex/bison/gperf generated tables, - seven files whose file-scope macros must not leak into unity siblings, - the 30 heavy template-instantiation TUs (>15s wall or >2.2GB RSS in the compile bench: the min_max/collect/topn/percentile aggregate family, in.cpp, multiply.cpp, function_array_aggregation.cpp, ...) whose per-file codegen would only stack into jumbo poles, - three files that tests compile a second time by #including the .cpp (function_variant_element.cpp, uuid.cpp, function_jsonb_transform.cpp): the test object must shadow a never-pulled archive member, but a unity batch is pulled in for its siblings and the linker sees a duplicate strong definition (caught by the BE UT lane). Measured on the validation build (-j14 + PCH, on the 32-batch layout before the three test-shadowed files were opted out): the unity TUs take 533s of slot time under full parallel load where their members summed to ~1418s in the cold bench; the largest unity TU compiles in 25s / 1.95GB RSS standalone. The archive keeps all external symbols (one weak linkonce_odr instantiation dedups away; the DictGetManyState rename adds its shared_ptr machinery under the new name) and shrinks 2721MB -> 1392MB. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ND7L1ZVTJf91TBpLwYSqct
morningman
force-pushed
the
be-build-opt-2-unity-exec-exprs
branch
from
August 14, 2026 10:12
9f486b7 to
d5b8f0b
Compare
Contributor
Author
|
run buildall |
Contributor
TPC-H: Total hot run time: 18071 ms |
Contributor
TPC-DS: Total hot run time: 86823 ms |
Contributor
ClickBench: Total hot run time: 14.83 s |
Contributor
BE UT Coverage ReportIncrement line coverage Increment coverage report
|
Contributor
Author
|
run p0 |
Contributor
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
CalvinKirs
approved these changes
Aug 14, 2026
Contributor
|
PR approved by at least one committer and no changes requested. |
Contributor
|
PR approved by anyone and no changes requested. |
Contributor
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
3 tasks
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: #66510, #66712
Problem Summary:
Same mechanism as #66712: most of the cold-build cost of glue-heavy targets is
re-parsing the shared header closure once per small
.cpp, and CMake'sUNITY_BUILDmakes a batch pay that parse once. What is new here is the scale —Exec (174 TUs,
libExec.a2450 MB) and Exprs (287 TUs,libExprs.a2721 MB) arethe two heaviest targets in the tree, and their glue shares the heaviest closures
(
operator.h/dependency.hfor Exec; the vexpr/factory closure for Exprs).The four commits:
change):
file_scanner.cpp/file_scanner_v2.cppboth defined the Icebergdelete content codes and
is_iceberg_position_deletes_sys_table()in anonymousnamespaces — the shared trio moves to
iceberg_scan_semantics.h(
file_scanner_v2_test.cppcarried a third copy, kept file-local by [fix](build) Fix the BE unit-test and benchmark build on macOS arm64 #66615because this header move had not landed yet; it now uses the header too).
vtablet_writer.cpp/vtablet_writer_v2.cppboth defined a file-scopeCLOSE_WAIT_EVENT_FALLBACK_MS— scoped intoIndexChannelandVTabletWriterV2; v2's file-scopeon_partitions_created()trampoline renamedon_partitions_created_v2(the two functions cast to different writer types).exchange_sink_operator.cpp's namespace-scopetimer_namerenamedwait_for_dependency_timer_name(shadowed unity siblings' locals under-Wshadow -Werror).≤12 sources, gated on
ENABLE_UNITY_BUILDlike the pilot targets. Opted out:five files whose file-scope macros must not leak into siblings, plus the two
heaviest template-instantiation TUs (
operator.cpp,hashjoin_build_sink.cpp)which would dominate any batch they join;
scan_operator.cppis both.dictionary_factory.hhad no include guard at all — any TU reaching itthrough two include paths fails with a class redefinition, and under unity the
clang error recovery poisoned unrelated batch members with spurious
-Warray-boundsdiagnostics. Now#pragma once. Andfunction_dict_get_many.cpphad copy-pasted theDictGetStatestruct fromfunction_dict_get.cppat namespace scope — renamedDictGetManyStateso thetwo TUs can share a batch. And
function_variant_element_v2.cppkeptOwnedPathSegmentin an anonymous namespace while using it as a field of theexternally-visible
ResolvedVariantElementV2Path::Impl— gcc's-Wsubobject-linkage(-Werror) rejects exactly that once the file is#included into a unity batch instead of being the main file of its TU(clang has no such warning); the struct moves to namespace scope.
sources, same switch. Opted out: the flex/bison/gperf generated tables, seven
macro-leaking files, the 30 heavy template-instantiation TUs (>15 s wall or
Measured results
All numbers from the development branch this series is split from, macOS arm64 +
clang 20,
-j14,ENABLE_PCH=ON, cold builds, back-to-back A/B. The baseline isthe #66712 state of that branch (10m16s), so the two waves compose with the pilot:
libExec.a2450 MB → 641 MBlibExprs.a2721 MB → 1392 MBJumbo-TU envelope: the largest Exec unity TU compiles in 32 s / 2.6 GB RSS
standalone, the largest Exprs one in 25 s / 1.95 GB — both below the largest
existing individual TU in the tree (3.9 GB), so
-jNmemory envelopes areunchanged.
Risk and verification
constants deduplicated with identical values, one constant scoped into its class,
two renames, one
#pragma once. No logic change.external defined symbols — three weak linkonce_odr template instantiations dedup
away, which is the point of unity, not a loss. Exprs likewise (one weak
instantiation dedups; the
DictGetManyStaterename carries itsshared_ptrmachinery under the new name).
(macOS arm64, clang 20,
ENABLE_PCH=ON,ENABLE_UNITY_BUILD=ON):7981/7981 ninja edges, zero failures,
doris_belinks (325 MB). Execproduces exactly 14 unity TUs and Exprs exactly 31, as advertised. This includes
pipeline/rec_cte_shared_state.cpp, added upstream after the waves weremeasured — it lands inside an Exec unity batch via the existing
GLOB_RECURSEwith zero CMakeLists edits, which is the intended maintenance story.
fixed in the current revision.
.cppa second time by#includeing it (function_variant_element_test,function_uuid_test,function_json_object_flatten_test). Pre-unity thislinked only by archive-member selectivity: the test object defines the
symbols first and the library member is never pulled. A unity batch,
however, is pulled in for its siblings and brings a second strong
definition. The three
#included files(
function_variant_element.cpp,uuid.cpp,function_jsonb_transform.cpp)are now
SKIP_UNITY_BUILD_INCLUSION— individual archive members restoreexactly the shadowing semantics master links with today.
-Werror=subobject-linkage:function_variant_element_v2.cppheldOwnedPathSegmentin an anonymousnamespace as a field type of the externally-visible
...Path::Impl. gcconly raises
-Wsubobject-linkagewhen the definition sits in an#included file — which is what unity turns a.cppinto; clang has nosuch warning, so every local build was green. The struct moves to
namespace scope (name unique to the TU); fixed at the source rather than
SKIPped.
ENABLE_UNITY_BUILD=OFFdrops all 51unity_*.cxxentries fromcompile_commands.json(Exec 14, Exprs 31, the [opt](build) Add ENABLE_UNITY_BUILD and pilot unity builds on three glue targets #66712 pilots 6) and the TUcount goes 8464 → 9042 — the batches return to exactly their 629 member files.
Reconfiguring back ON restores exactly the same 51 batches. The switch
semantics themselves (including winning over a stale
CMAKE_UNITY_BUILDcache) were established in [opt](build) Add ENABLE_UNITY_BUILD and pilot unity builds on three glue targets #66712.
since 2026-08-08, through repeated full-tree builds and the BE UT builds that
verified [opt](build) Cut three more waves of hot include edges in the BE header graph #66672 (the UT binaries link against these same target libraries).
file_scanner_v2_test.cpphunk was compile-verified standalone against thisbranch (
-fsyntax-onlywith the test TU's full include closure).Proactive disclosure
build and measurement above is macOS arm64 + clang 20. With
ENABLE_UNITY_BUILDdefaulting ON since [opt](build) Add ENABLE_UNITY_BUILD and pilot unity builds on three glue targets #66712, the Linux compile lanes and every regression pipeline
in this PR's CI run against unity Exec/Exprs — that is the validation, and the
first round proved it works: the BE UT and gcc lanes each caught one real
unity interaction (detailed above), fixed in this revision. Escape hatches, in order:
per-user
ENABLE_UNITY_BUILD=OFF, per-fileSKIP_UNITY_BUILD_INCLUSION, or aone-line default flip.
.cppinside abatch recompiles the whole batch (≤12 sources for Exec, ≤8 for Exprs; a batch
compiles in ~25–32 s). This is why the heaviest, most-edited TUs
(
operator.cpp,hashjoin_build_sink.cpp, the aggregate families,in.cpp,multiply.cpp, …) are deliberately SKIPped and keep per-file granularity, andENABLE_UNITY_BUILD=OFFrestores it everywhere.stay individual on purpose — generated parsers (flex/bison/gperf), files whose
file-scope macros would leak into siblings, and the heavy codegen TUs where
merging saves no closure parse worth the jumbo-TU cost. A future file whose
file-scope symbols clash inside a unity TU opts out the same one-line way.
30 heavy Exprs SKIPs mean the headline per-target ratios (2.6× Exec slot time)
are earned on the batched part; the SKIPped monsters keep their cost and their
per-file granularity.
Release note
None
Check List (For Author)
Test
Behavior changed:
Does this need documentation?
🤖 Generated with Claude Code
https://claude.ai/code/session_01Gdfkk7RqgD5e3Uv7bTM3NV