[opt](build) 1/4: Speed up BE full build ~22% by cutting hot-header include edges - #66400
Open
morningman wants to merge 17 commits into
Open
[opt](build) 1/4: Speed up BE full build ~22% by cutting hot-header include edges#66400morningman wants to merge 17 commits into
morningman wants to merge 17 commits into
Conversation
Add a timing framework to locate where BE build time goes: - build.sh --compile-bench: cold, cache-free, BE-only benchmark build. Uses a dedicated build dir (recreated every run), disables ccache by replacing the compiler launcher with a timing wrapper, skips FE/cloud/ java-extensions/packaging, and records per-phase timings (gensrc, submodules, configure, build). - build-support/compile-bench/cc-timing-wrapper.py: compiler/linker launcher that records wall/user/sys time and peak RSS per invocation with exit codes and diagnostics passed through untouched. - build-support/compile-bench/report.py: generates report.txt and summary.json per run (slowest TUs, per-directory rollups, critical-path tail from .ninja_log, optional clang -ftime-trace aggregation of header parse and template instantiation costs; handles both clang<20 complete Source events and clang>=20 begin/end pairs), plus a compare subcommand to quantify the effect of changes between two runs. - COMPILE_BENCH_TRACE=ON additionally compiles with -ftime-trace. Results land in be/compile-bench-results/<timestamp>/ (gitignored). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
cut_impact.py: simulate removing one or more #include edges against the real ninja dep closure of the last bench build. Reports which TUs lose which headers, which files textually reference lost symbols (seeding advice), and supports edge / audit / why subcommands. syntax_sweep.py: parallel -fsyntax-only replay of compile_commands.json to validate include-structure changes across every TU without touching the ninja build state. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ne sinks Pure-additive preparation so that workload_group.h (which carries gen_cpp/BackendService_types.h — the whole thrift type universe — plus backend_options.h and network_util.h) can later be cut out of the hot headers that reach ~1000 TUs. No include edge is removed in this commit and there is no behavior change: - Add runtime/workload_group/workload_group_fwd.h; move the WorkloadGroupPtr alias there (workload_group.h now includes it) - Sink ThreadMemTrackerMgr::try_reserve/shrink_reserved bodies to the .cpp (they dereference WorkloadGroup in inline code) - Sink MemTableWriter::workload_group_id() body to the .cpp (same reason) - Forward-declare QueryContext / TQueryStatistics / TReportExecStatusParams in the headers that were freeloading the declarations transitively - Seed direct includes (workload_group.h / backend_options.h / FrontendService_types.h) into every TU that actually uses those types but received them only through the soon-to-be-cut chains, including the LIMIT_LOCAL_SCAN_IO / LIMIT_REMOTE_SCAN_IO macro expansion sites Verified with build-support/compile-bench/syntax_sweep.py: 1358/1358 TUs pass -fsyntax-only with the full patch series applied. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Cut nine include edges that spread the thrift type universe and the schema scanner stack into nearly every TU (all seeds landed in the previous commit; this commit only removes edges and locks them): - exec_env.h: drop information_schema/schema_routine_load_job_scanner.h. Zero references in exec_env.h (RoutineLoadTaskExecutor is already forward-declared); the line was carried over mechanically in 2eef794. - Swap workload_group.h -> workload_group_fwd.h in thread_mem_tracker_mgr.h, resource_context.h, runtime_state.h, query_context.h and memtable_memory_limiter.h (signature-only users). - Drop dead workload_group.h includes from cpu_context.h, task_scheduler.h and vdata_stream_recvr.h (zero references). - Add three check-header-deps.py rules so the edges cannot silently return: exec_env.h !-> information_schema/, thread_context.h !-> workload_group/, runtime_state.h !-> workload_group/. Simulated against the last compile-bench dependency graph (cut_impact.py): workload_group.h leaves 1051 TUs; backend_options.h 979; network_util.h 975; BackendService/DorisExternalService_types.h 865; FrontendService/ MasterService_types.h 638; schema scanner headers ~1050. Verified with syntax_sweep.py: 1358/1358 TUs pass -fsyntax-only. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Pure-additive preparation so that four include edges can later be cut from runtime/exec_env.h, which reaches ~1060 TUs: io/cache/fs_file_cache_storage.h (carries gen_cpp/internal_service.pb.h, descriptors.pb.h and the io/fs family), runtime/frontend_info.h (carries gen_cpp/HeartbeatService_types.h and AgentService_types.h), runtime/cluster_info.h and load/memtable/memtable_memory_limiter.h. No include edge is removed in this commit and there is no behavior change: - exec_env.h: forward-declare io::FDCache, MemTableMemoryLimiter, ClusterInfo, FrontendInfo and the thrift-generated TFrontendInfo / TNetworkAddress; include <set> directly (init()'s std::set parameter was riding on the thrift headers) - Sink set_file_cache_open_fd_cache / set_memtable_memory_limiter bodies to the .cpp: assigning/resetting the unique_ptr destroys the old pointee, which would require the complete type in every includer (same reasoning as the existing out-of-line set_tmp_file_dir) - Hold the frontends map behind std::unique_ptr<std::map<...>>: std::map requires a complete mapped type, and FrontendInfo embeds TFrontendInfo by value; the map is allocated in the (already out-of-line) constructor - fragment_mgr.h, vdata_stream_recvr.h: forward-declare FrontendInfo / PTransmitDataParams (declaration-only uses that freeloaded off exec_env.h) - format/parquet/parquet_predicate.h, storage/rowset/rowset_writer_context.h: include io/fs/file_reader.h / local_file_system.h their inline code dereferences - inverted_index_common_impl.h: wrap <CLucene.h> in a -Wconversion suppression; whether its first expansion lands inside someone else's suppressed region depends on include order, so suppress it deliberately - Seed direct includes (runtime/cluster_info.h x44, load/memtable/memtable_memory_limiter.h, io/fs/local_file_system.h, io/cache/block_file_cache_factory.h, fs_file_cache_storage.h, <concurrentqueue.h>) into every TU that uses those types but received them only through the soon-to-be-cut chains Verified with build-support/compile-bench/syntax_sweep.py: 1358/1358 TUs pass -fsyntax-only with the full patch series applied. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ec_env.h Cut four include edges from runtime/exec_env.h (all seeds landed in the previous commit; this commit only removes edges and locks them): - io/cache/fs_file_cache_storage.h: ExecEnv holds io::FDCache behind a unique_ptr with the setter defined out of line, and FileCacheFactory / PBackendService_Stub were already forward-declared. Simulated on the post-P1.1a dependency graph (cut_impact.py): 1057 TUs stop seeing the header; gen_cpp/internal_service.pb.h leaves 656 TUs, descriptors.pb.h 650, io/fs/local_file_system.h 886, io/fs/file_writer.h 850, the block_file_cache family ~526 each - runtime/frontend_info.h: the frontends map now lives behind a unique_ptr, everything else is declaration-only. HeartbeatService_types.h leaves 242 TUs, AgentService_types.h 214 - load/memtable/memtable_memory_limiter.h (1048 TUs) and runtime/cluster_info.h (1056 TUs): pointer/accessor-only uses, forward-declared - Add four check-header-deps.py rules so the edges cannot silently return: exec_env.h !-> io/cache/ (except file_cache_common.h, which storage/options.h legitimately needs for CachePath), !-> load/memtable/, !-> runtime/frontend_info.h, !-> runtime/cluster_info.h storage/tablet/tablet_fwd.h was audited and deliberately kept: it is a pure forward-declaration header, cutting it saves nothing. Verified with build-support/compile-bench/syntax_sweep.py: 1358/1358 TUs pass -fsyntax-only; build-support/check-header-deps.py: 8/8 rules pass. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Pure-additive preparation so that three include edges can later be cut from runtime/exec_env.h, which reaches ~1060 TUs: gen_cpp/olap_file.pb.h (carries olap_common.pb.h), util/threadpool.h (carries thread.h, common/metrics/metrics.h, agent/cgroup_cpu_ctl.h and the blocking-queue family) and storage/options.h (carries gen_cpp/Types_types.h and io/cache/file_cache_common.h). No include edge is removed in this commit and there is no behavior change: - exec_env.h: forward-declare ThreadPool, StorePath and CachePath. Every pool member is a unique_ptr<ThreadPool> with a .get() accessor, and the store paths live in std::vector members behind reference-returning accessors, so declarations suffice - Sink set_non_block_close_thread_pool / set_s3_file_upload_thread_pool bodies to the .cpp: assigning the unique_ptr destroys the old pointee, which would require ThreadPool to be complete in every includer (same reasoning as the existing out-of-line setters); exec_env.cpp includes util/threadpool.h directly - storage/segment/segment.h: include io/cache/file_cache_common.h -- file_cache_key() returns io::UInt128Wrapper by value - workload_sched_policy_mgr.h: include <shared_mutex> and forward-declare Thread (both rode in through exec_env.h -> util/threadpool.h); the .cpp includes util/thread.h for Thread::create - common/signal_handler.h: include <butil/macros.h> for ARRAYSIZE_UNSAFE - util/brpc_client_cache.h: include util/defer_op.h for Defer - function_java_udf.cpp (ThreadPool::submit_func), paimon_jni_reader.cpp and wal_manager.cpp (StorePath member access), python_udf_meta.cpp (<rapidjson/document.h>): include what their code dereferences Verified with build-support/compile-bench/syntax_sweep.py: 1358/1358 TUs pass -fsyntax-only with the full patch series applied. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ec_env.h Cut three include edges from runtime/exec_env.h (all seeds landed in the previous commit; this commit only removes edges and locks them): - gen_cpp/olap_file.pb.h: dead include -- ExecEnv names nothing from it. Simulated on the post-P1.2 dependency graph (cut_impact.py): 226 TUs stop seeing olap_file.pb.h and olap_common.pb.h - util/threadpool.h: every pool is a unique_ptr<ThreadPool> with .get() accessors, the assigning setters are now out of line. 515 TUs stop seeing threadpool.h, agent/cgroup_cpu_ctl.h, work_thread_pool.hpp, thread_group.h and the blocking-queue family; util/thread.h leaves 500 TUs, common/metrics/metrics.h and util/histogram.h leave 248 each - storage/options.h: StorePath/CachePath appear only inside std::vector members and reference-returning accessors, so forward declarations suffice. 837 TUs stop seeing options.h; io/cache/file_cache_common.h leaves 519 TUs, gen_cpp/Types_types.h stops riding this edge - check-header-deps.py: also capture angle-bracket <gen_cpp/...> includes so a rule can name generated headers; add three rules -- exec_env.h !-> util/threadpool.h, !-> storage/options.h and !-> gen_cpp/ (except the two carriers of Status, Status_types.h and types.pb.h) -- and drop the io/cache/file_cache_common.h exception, which the options.h cut makes unnecessary After the cut, exec_env.h's own include closure is 13 project headers (config.h, multi_version.h, status.h, tablet_fwd.h and their subtrees). Verified with build-support/compile-bench/syntax_sweep.py: 1358/1358 TUs pass -fsyntax-only; build-support/check-header-deps.py: 11/11 rules pass. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…sinks, seeds Pure-additive preparation so that three include edges can later be cut from runtime/runtime_state.h (~1067 TUs) and runtime/thread_context.h (~1008 TUs): io/fs/s3_file_system.h (carries util/s3_util.h, the AWS SDK surface and gen_cpp/cloud.pb.h -- 5.8MB of preprocessed payload per TU), runtime/workload_management/resource_context.h (carries the whole workload_management family plus gen_cpp/data.pb.h -- 1.4MB) and runtime/exec_env.h. No include edge is removed in this commit and there is no behavior change: - runtime_state.h: forward-declare io::S3FileSystem. _s3_error_fs is a shared_ptr member whose every dereference already lives in runtime_state.cpp (which already includes s3_file_system.h) - thread_context.h: forward-declare ResourceContext; sink ThreadContext::attach_task to the .cpp (called once per task attach); split ThreadContext::resource_ctx() -- the attached hot path stays inline (copying a shared_ptr of an incomplete type is legal), the orphan fallback moves out of line as _make_orphan_resource_ctx(), which was the only place this header dereferenced ResourceContext and ExecEnv outside macro bodies - thread_context.cpp: include exec_env.h and resource_context.h directly - scanner_context.h: sink ScanTask ctor/dtor to scanner_context.cpp -- their bodies dereference ResourceContext via thread_context() and rode both thread_context.h and resource_context.h transitively; forward-declare ResourceContext for the shared_ptr members - data_type_timestamptz.h: sink to_pb_column_meta to the .cpp -- its body dereferences PColumnMeta, which rode in through resource_context.h -> gen_cpp/data.pb.h - LIMIT_LOCAL/REMOTE_SCAN_IO expansion sites (buffered_reader.cpp, local_file_reader.cpp, s3_file_reader.cpp, hdfs_file_reader.cpp, peer_file_cache_reader.cpp): include resource_context.h (and io_throttle.h where missing) -- the macros dereference resource_ctx()->workload_group() at the expansion point - vectorized_agg_fn.h (ENABLE_FACTORY_CREATOR needs common/factory_creator.h), query_cache.h (TScanRangeParams / TQueryCacheParam need gen_cpp/PaloInternalService_types.h and QueryCache_types.h), function_java_udf.cpp (<future> for std::packaged_task, which rode the AWS SDK headers), memtable.cpp / memtable_flush_executor.cpp / memtable_writer.cpp (ResourceContext dereferences): include what the code actually uses instead of riding the soon-to-be-cut edges Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…hot headers Cut four include edges (all seeds landed in the previous commit; this commit only removes edges and locks them). Edge priority came from differential payload measurement: preprocess a probe TU with and without each edge and rank by affected-TUs x payload-bytes -- which also showed that the other fat-looking edges of these two headers (thread_mem_tracker_mgr.h at 1006 TUs, task_execution_context.h at 635, debug_util.h at 746) share their whole subtree with other paths and are not worth cutting (0.00-0.03MB differential): - runtime_state.h -/-> io/fs/s3_file_system.h: _s3_error_fs is a forward-declared shared_ptr, dereferenced only in runtime_state.cpp. Differential payload 5.8MB of preprocessed source per TU; on the post-P1.3 graph 711 TUs stop seeing s3_file_system.h, util/s3_util.h and cpp/aws_common.h (685 TUs, the AWS SDK surface), obj_storage_client.h (683), token_bucket_rate_limiter.h (684), gen_cpp/cloud.pb.h (585) and io/fs/remote_file_system.h (414). runtime_state.h's preprocessed size drops 22.1MB -> 16.3MB - thread_context.h -/-> runtime/workload_management/resource_context.h: ResourceContext is forward-declared; attach_task and the orphan fallback are out of line. Differential payload 1.4MB; 809 TUs stop seeing the workload_management family (resource/cpu/io/memory context, task_controller, io_throttle), 586 stop seeing gen_cpp/data.pb.h and 189 stop seeing gen_cpp/PaloInternalService_types.h. thread_context.h's preprocessed size drops 16.5MB -> 15.1MB - thread_context.h -/-> runtime/exec_env.h: after the resource_ctx() split the header only names ExecEnv inside macro bodies, which expand at call sites (audit: zero TUs lose exec_env.h through this cut -- every includer has another path; the edge only cost graph structure). The transitive path via thread_mem_tracker_mgr.h remains, so no layering rule is added for this edge - resource_context.h -/-> gen_cpp/data.pb.h: dead include -- the header references no data.pb symbol (TQueryStatistics is thrift and already forward-declared). Independently of the thread_context cut, 595 TUs stop seeing data.pb.h (and 26 gen_cpp/segment_v2.pb.h) through this edge - check-header-deps.py: four new rules -- runtime_state.h !-> io/fs/s3_file_system.h; thread_context.h !-> runtime/workload_management/; thread_context.h !-> gen_cpp/ (except the six status/types/profile carriers Status_types.h, types.pb.h, Types_types.h, Metrics_types.h, RuntimeProfile_types.h, runtime_profile.pb.h); resource_context.h !-> gen_cpp/data.pb.h Verified with build-support/compile-bench/syntax_sweep.py: 1358/1358 TUs pass -fsyntax-only; build-support/check-header-deps.py: 15/15 rules pass. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
morningman
requested review from
Gabriel39,
airborne12,
csun5285,
deardeng,
eldenmoon,
gavinchou,
liaoxin01 and
yiguolei
as code owners
August 3, 2026 23:22
Contributor
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
morningman
changed the title
Contributor
Author
|
run buildall |
Header slimming cut the transitive include chain that used to bring <condition_variable> into global_memory_arbitrator.h on Linux libstdc++ (macOS libc++ provides it transitively via <mutex>/<thread>, which is why local builds passed). Add the std headers each file uses directly; also fix six more headers with the same latent issue. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Contributor
Author
|
run buildall |
Contributor
TPC-H: Total hot run time: 29006 ms |
Contributor
TPC-DS: Total hot run time: 169145 ms |
Contributor
ClickBench: Total hot run time: 23.84 s |
Same root cause as 7097715: these tests used S3Conf and io::S3FileSystem via transitive includes that header slimming removed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
std::condition_variable in block_file_cache_test and std::shared_mutex in scanner_context_test relied on transitive includes that differ between libc++ (macOS) and libstdc++ (Linux CI). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Contributor
Author
|
run buildall |
The BE UT build failed on two TUs using io::global_local_filesystem() without including io/fs/local_file_system.h, previously reachable via runtime_state.h -> s3_file_system.h. Since the failed build stopped before compiling ~780 test TUs, diffed every remaining TU's include closure against the pre-cut tree and checked the symbols it uses, fixing the whole family in one pass: - io/fs/local_file_system.h: 9 files (incl. both CI failures) - load/memtable/memtable_memory_limiter.h: 7 files (new ... needs the complete type; the exec_env.h edge is gone) - runtime/workload_management/resource_context.h: 2 files - io/cache/fs_file_cache_storage.h (FDCache), service/backend_options.h: 2 files each - runtime/cluster_info.h, runtime/frontend_info.h, util/threadpool.h (run_all_tests.cpp), runtime/workload_group/workload_group_fwd.h: 1 file each Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Contributor
Author
|
run buildall |
…complete types FileWriterPtr (unique_ptr<FileWriter>) and FileReaderSPtr (shared_ptr<FileReader>) are declared in file_reader_writer_fwd.h, so the names resolve, but destroying the unique_ptr or calling members through either alias needs the complete type, which used to arrive transitively via runtime_state.h -> s3_file_system.h before that edge was cut. Swept the remaining not-yet-compiled test TUs for both alias kinds (unique_ptr: any use; shared_ptr: declared variable later dereferenced) and calibrated the scan against the 588 TUs that already compile clean: - io/fs/file_writer.h: native_reader_writer_test (the CI failure), vorc_transformer_test, format_v2 native_reader_test, metadata_adder_test - io/fs/file_reader.h: io/client/s3_file_system_test (the local run-be-ut.sh failure; the file already included file_writer.h and file_system.h but relied on the cut chain for file_reader.h) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Contributor
Author
|
run buildall |
Contributor
|
/review |
Contributor
TPC-H: Total hot run time: 28891 ms |
Contributor
TPC-DS: Total hot run time: 166735 ms |
Contributor
ClickBench: Total hot run time: 23.79 s |
Contributor
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Request changes. The production header slimming and out-of-line moves appear behavior-preserving at the exact head: Linux COMPILE and BE UT pass, the 15 dependency rules pass when invoked manually, and the macOS job compiled all affected TUs before an unrelated final-link --exclude-libs failure. The new benchmark/validation tooling still has seven correctness gaps covering documented clean behavior and path isolation, target normalization, silent empty/failed analyses, comparison integrity, and actual enforcement of the dependency guard; each is detailed inline.
Critical checkpoints: the include-cut goal is implemented with direct seeds and complete-type boundaries, and no transaction, storage format, wire protocol, configuration, or FE/BE variable compatibility change is present. Existing runtime concurrency, _frontends_lock protection, memory-reservation rollback, scan-task cleanup, and object lifecycles remain intact. The source changes are broad but mechanically focused; current compile/unit-test evidence covers runtime linkage and platform-sensitive headers. The cold-build performance claim is appropriately a build-time claim, but the new tools must reject invalid or incomparable inputs before their results can serve as evidence. No additional user review focus was provided. All mandated reviewer lanes converged on this frozen finding set with no unresolved candidate.
| CLOUD_EXTRA_CMAKE_ARGS+=("-D${feature_name}_MODULE_DIR=${CLOUD_EXTRA_MODULE_PATHS[i]}") | ||
| done | ||
|
|
||
| if [[ "${COMPILE_BENCH}" -eq 1 ]]; then |
Contributor
There was a problem hiding this comment.
[P2] Normalize compile-bench before the clean-only path
The documented ./build.sh --compile-bench --clean reaches the clean-only branch at lines 520-524 before this block, so it deletes the normal gensrc/BE/FE artifacts and exits without running a benchmark. Adding --be avoids that exit but still calls clean_be() at line 839 before the benchmark directory is selected, deleting the normal be/build_<Type> and be/output. Please move compile-bench normalization ahead of the clean-only dispatch and ensure benchmark cleanup never targets the ordinary build/output paths.
| else: # block header "<target>: #deps N, ... (VALID|STALE)" or noise | ||
| expect_source = line.rstrip().endswith("(VALID)") | ||
| cur = None | ||
| proc.wait() |
Contributor
There was a problem hiding this comment.
[P2] Fail when the dependency graph cannot be loaded
ninja -t deps stderr is suppressed and this return code is ignored, so a missing, stale, or Make-generated build directory is reported as 0 TUs / 0 affected with exit code 0. I reproduced that with an explicitly missing --build-dir. That false-negative result can make an unsafe include cut look harmless; please reject a failed dependency command and an empty valid-TU set instead of emitting a successful report.
| help="write full stderr of every failing TU to this file") | ||
| args = ap.parse_args() | ||
|
|
||
| src_prefix = os.path.join(REPO_ROOT, "be", "src") + os.sep |
Contributor
There was a problem hiding this comment.
[P2] Do not report an empty source-only sweep as full validation
This selection excludes every be/test TU, while compile-bench also configures MAKE_TEST=OFF; it then returns success even when the selection is empty (for example, after a misspelled --filter). Header cuts affect test-only consumers too—this PR itself adds many required test includes—so the advertised all-TU validation can pass without checking them. Please fail on zero selected TUs and either include a BE-UT compile database or explicitly scope the tool and require the prescribed test build for test consumers.
| return lines, summary | ||
|
|
||
|
|
||
| def cmd_report(args): |
Contributor
There was a problem hiding this comment.
[P2] Fail a successful benchmark when timing data is absent
This only verifies that run_dir exists. With missing metadata/phases/compile log, the readers return empty values, the report says build status : ok with zero compile units, and the command exits 0. Because timing-log writes and report failures are also intentionally swallowed upstream, a successful build can therefore produce no usable benchmark while the overall command still succeeds. Please validate the required inputs/nonempty TU data and propagate report failure when the benchmark build itself succeeded.
| return json.load(fh) | ||
|
|
||
|
|
||
| def cmd_compare(args): |
Contributor
There was a problem hiding this comment.
[P2] Reject failed or incompatible benchmark comparisons
cmd_compare() ignores each summary's build_status and recorded invariants such as toolchain, build type, parallelism, PCH, generator, and AVX. It can therefore present a partial failed build—or a materially different configuration—as a speedup. Please reject incomplete/failed runs and mismatched compile-affecting metadata by default, record the currently omitted target/extra-module flags, and require an explicit override for intentional heterogeneous comparisons.
| BUILD_HIVE_UDF=0 | ||
| BUILD_BE_JAVA_EXTENSIONS=0 | ||
| BUILD_BE_CDC_CLIENT=0 | ||
| OUTPUT_BE_BINARY=0 |
Contributor
There was a problem hiding this comment.
[P2] Normalize every target selector in compile-bench mode
This block clears FE/cloud/package flags but leaves BUILD_BENCHMARK, the meta/index/cache tool flags, and BUILD_TASK_EXECUTOR_SIMULATOR active. For example, --compile-bench --benchmark defines BE_TEST/BE_BENCHMARK, omits doris_be, and builds benchmark_test, so it is no longer measuring the advertised normal BE target and cannot be compared with the baseline. Please reject these combinations or reset all auxiliary target selectors, and record the effective target set in the run metadata.
| "index writer stack (and CLucene) in front of most of the backend", | ||
| ), | ||
| ( | ||
| "runtime/exec_env.h", |
Contributor
There was a problem hiding this comment.
[P2] Wire the dependency guard into an enforced check
These new rules pass when the script is run manually, but a repository-wide search finds no build, presubmit, or workflow invocation of check-header-deps.py; its only reference is its own usage text. As a result, the forbidden edges can return silently despite the stated goal of locking the cuts. Please invoke this checker from an always-run validation path (and ideally add focused rule tests) so a regression actually fails CI.
Apple's ld rejects --exclude-libs (a GNU ld option), so the final link of doris_be fails on macOS with 'ld: unknown options: --exclude-libs' since the flag was introduced. macOS also does not need the symbol hiding: dyld's two-level namespace already binds each loaded library to the copy it was linked against. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Contributor
Author
|
run buildall |
Contributor
TPC-H: Total hot run time: 28918 ms |
Contributor
TPC-DS: Total hot run time: 166267 ms |
Contributor
ClickBench: Total hot run time: 23.77 s |
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
3 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?
Issue Number: close #xxx
Related PR: #xxx
Problem Summary:
A cold full BE build spends most of its wall clock re-parsing a handful of
hub headers in nearly every TU.
runtime/exec_env.h,runtime/runtime_state.hand
runtime/thread_context.heach reached ~1000 of the ~1400 be/src TUs anddragged multi-MB include subtrees (thrift/protobuf type universes, the AWS SDK
surface, the workload_management family) into every one of them.
This PR adds a measurement harness and then cuts the high-payload include
edges from those hub headers, in four prepare+cut commit pairs. Measured with
the included bench mode (cold, cache-free,
-j5, Apple M5 Pro, clang 20):Total: 39m21s -> 30m48s wall (-21.7%). Per-file compare shows 672-1065
files faster per wave and zero per-file compile-time regressions.
Key touch-count reductions (TUs that stop parsing a header):
FrontendService_types.h1086->459,BackendService_types.h->225,io/fs/s3_file_system.h+ AWS SDK 769->58,gen_cpp/cloud.pb.h->192,workload_management/resource_context.h1015->214,gen_cpp/data.pb.h->443,util/threadpool.h-515 TUs,storage/options.h-837 TUs.What is in the series (10 commits, meant to be read pairwise):
build.sh --compile-bench: cold cache-free BE bench with per-TUwall/user/maxrss capture, report and A/B compare tooling
(
build-support/compile-bench/)cut_impact.py(blast-radius and reseedestimation from
ninja -t deps+ parsed include graph) andsyntax_sweep.py(parallel-fsyntax-onlyreplay of all TUs)declarations, out-of-line sinks of cold method bodies, direct includes for
TUs that free-rode the edge about to be cut); every "cut" commit only
removes the edges and locks them with new rules in
build-support/check-header-deps.py(15 rules total), so none of thecut edges can silently return.
The only code moved out of line: assigning ThreadPool setters,
ThreadContext::attach_task, the orphanResourceContextfallback ofresource_ctx()(hot attached path stays inline),ScanTaskctor/dtor andDataTypeTimeStampTz::to_pb_column_meta-- all cold paths.Release note
None
Check List (For Author)
Test
build-support/compile-bench/syntax_sweep.py: 1358/1358 be/src TUspass
-fsyntax-onlyafter every cut wavebuild-support/check-header-deps.py: 15/15 layering rules passsh build.sh --compile-benchbuilds succeed after every wave(five complete cold builds); per-file A/B compare via
build-support/compile-bench/report.py compare <old> <new>showszero compile-time regressions
Behavior changed:
Does this need documentation?
Check List (For Reviewer who merge this PR)
🤖 Generated with Claude Code