mirror of
https://github.com/apache/doris.git
synced 2026-11-04 07:31:17 +00:00
Mingyu Chen (Rayner)andClaude Fable 5
7933a82bdd
### What problem does this PR solve? Related PR: #66510 (the BE build-time series, #66615..#66826, that this PR locks in) Problem Summary: The build-time series cut BE cold builds by ~49% and took `olap_common.h` / `uid_util.h` off the everything-rebuilds line. Every one of those wins can regress **silently**: a stray include, a lost `extern template`, a header added to the PCH — the code still compiles, only every build afterwards is slower, and nothing reports it. `#66052` already showed the failure mode: the layering rules in `check-header-deps.py` only worked because the author happened to maintain them by hand; nothing in CI runs them. This PR turns each known regression vector into a **fail-loud, seconds-level gate that runs at configure time**, so a violation is a build error on every path that builds the BE (CI, `build.sh`, the UT build) with zero extra CI wiring — the same mounting pattern the FE side already uses for its architecture gates (`exec-maven-plugin` running `check-fe-core-metadata-funnel.sh` at `validate`). **Checks** (pure text scans, ~1.3s combined, zero build dependency, zero false positives on current master): | Gate | Regression it stops | |---|---| | Header layering rules (30, +6 new) | a hub header (e.g. `exec_env.h`, `function.h`, `pch.h`) re-growing an include edge into a subsystem that was deliberately cut | | Third-party ban table (new) | `fmt`/`boost`/`concurrentqueue.h`/`<ranges>` returning to headers whose bodies were moved out of line to contain template-instantiation cost (`<ranges>` in a src header also breaks the `-fno-access-control` UT build on libc++) | | Two-axis budgets (new) | the edges nobody names: light hubs must stay light (include-closure budget, zero slack), heavy payloads must not spread (transitive-reach baseline +10%); `--closure` / `--reach` subcommands rank the offending edges for the fix | | pch whitelist (new) | any quoted include added to `pch/pch.h` — the single most leveraged regression in the repo (touch one such header and the whole backend plus the PCH rebuild) | | `check-extern-template-pairing.py` (new) | the one **silent** extern-template failure: a `.cpp` keeps the explicit instantiation but the header loses the `extern` declaration, so every TU quietly re-instantiates; also the loud direction (declaration with no definition), reported at once instead of at link | | `check-unity-skip-coverage.py` (new) | a test `#include`-ing a src `.cpp` that is not opted out of unity batching — reported at configure with the exact skip entry to add, instead of as duplicate symbols at the end of the BE UT link | `build-support/check-build-hygiene.sh` runs all three scripts; `be/CMakeLists.txt` executes it at configure time. Escape hatches, by design: - `-DENABLE_BUILD_HYGIENE=OFF` (default ON; a missing `python3` is a loud FATAL naming this switch, never a silent skip); - every budget/whitelist/rule table lives in the scripts themselves, so the legal way past a gate is a **one-line reviewed diff in the same PR** — the gate is never a dead end. Every failure message carries the mechanism (why this edge is expensive) and the concrete fix path. **Found by the new gates on day one, fixed here:** `core/types.h` has carried five `extern template struct fmt::formatter<Decimal*>;` declarations with **no matching definition anywhere** since #53483 — every member is defined in-class, so the linker never noticed; deleted. Also made the FE funnel self-test's `sed -i` invocation portable to BSD/macOS. Blind spot, by design: instantiations expanded from macros (e.g. `DECLARE_OPERATOR` in `operator.cpp`) are invisible to the pairing scan on both sides — macro bodies are skipped; a comment at the site says so. `be/README.md` (new) documents the gates, what to do when each fires, and the `*_fwd.h` convention as the sanctioned way through a layering barrier. The same rules are encoded for AI-assisted development and review: `be/AGENTS.md` (new — include discipline while writing BE code + review checkpoints), `build-support/AGENTS.md` (new — maintenance discipline for the gate scripts themselves), an include-src-cpp checkpoint in `be/test/AGENTS.md`, and a Header Hygiene entry in the root `AGENTS.md`, so coding agents and the review pipeline apply them automatically. ### Release note None ### Check List (For Author) - Test - [x] Manual test (add detailed scripts or steps below) - `build-support/check-build-hygiene.sh` on master: all green, ~1.3s, zero false positives. - Self-tests `bash build-support/tests/run.sh` (7/7 PASS): red/green injection per gate family — each seeded violation turns red with the mechanism + fix in the message, restoring the file turns it green again. - Real configure on macOS arm64: green path passes the gate section; with an injected violation, configure dies at the gate in under a second with the full message; `-DENABLE_BUILD_HYGIENE=OFF` bypasses; a missing `python3` is a loud exit-2 FATAL naming the switch. - This PR's own Linux CI exercises the configure mount on the second platform. - Behavior changed: - [x] Yes. BE configure now fails fast (with mechanism + fix in the message) on header-hygiene violations; emergency bypass via `-DENABLE_BUILD_HYGIENE=OFF`. No runtime behavior change — the only source-code changes are comments and the deletion of five never-defined `extern template` declarations. - Does this need documentation? - [x] No. Developer-facing docs are included in-repo (`be/README.md`). 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01EyZL631YTeuNXa9WLjvk7s --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
4.6 KiB
4.6 KiB
Doris BE development notes
Compile-time hygiene gates
BE configure runs build-support/check-build-hygiene.sh (option
ENABLE_BUILD_HYGIENE, default ON): a set of seconds-level text checks that
keep the backend's compile-time invariants from regressing. They exist because
include-graph regressions are silent -- the code still compiles, only every
build afterwards is slower. The gates make them loud, in the first seconds of
configure, with the mechanism and the fix in the message.
What each gate guards, and what to do when it fires:
- Header layering rules (
check-header-deps.py). A hub header (exec_env.h,thread_context.h, ...) must not reach a named subsystem, because everything a hub includes is reparsed by the ~1000 TUs behind it. Fix: forward-declare the type in the header and include the real header in the.cpp; if only declarations are needed by many files, route them through a*_fwd.h(see below). A genuinely leaf-like header can be added to the rule's exception set -- in the same PR, with the reasoning in the commit message. - Third-party bans. A few headers deliberately moved their
fmt/boost/<ranges>-using bodies out of line; the ban keeps the template machinery from coming back. Fix: put the code that needs the library into the matching.cpp. - Closure / reach budgets. The safety net for edges no rule names: light
hubs must stay light (
--closure <header>lists what grew), heavy payloads must not spread (--reach <header>ranks the spreading edges). Fix: cut the new edge, or -- when the growth is intended -- bump the number in the budget table in the same PR and say why in the commit message. The diff of the budget table is the review signal; the gate is never a dead end. - PCH whitelist.
pch/pch.h's quoted includes are pinned exactly: every header on the PCH rebuilds the whole backend when touched. Adding one is almost never right; if it is, change the whitelist in the same PR. - extern template pairing (
check-extern-template-pairing.py). Everyextern templatedeclaration in a header must have its explicit instantiation definition in a.cpp, and -- within a family that uses externs -- every definition must have its declaration. The missing-extern direction is the one silent case: it compiles and links, every TU just quietly re-instantiates the specialization. Fix: keep both sides in sync, spelling the template arguments the same way. - Unity-skip coverage (
check-unity-skip-coverage.py). A test that#includes a be/src.cppneeds that file opted out of unity batching (doris_skip_unity_inclusion), otherwise the BE UT link fails with duplicate symbols an hour later. The error names the exact entry to add.
The gates are python3 sources and resolve their own interpreter (python3,
then python, then /usr/libexec/platform-python, the RHEL8/AlmaLinux8 system
Python that the build-env image ships in place of /usr/bin/python3). They
deliberately ignore PYTHON, which env.sh exports as the interpreter the
build uses for code generation and which is python2 by default. Set
BUILD_HYGIENE_PYTHON to pin a specific interpreter.
Escape hatch for emergencies: configure with -DENABLE_BUILD_HYGIENE=OFF.
Self-tests live in build-support/tests/ (run.sh).
The *_fwd.h convention
*_fwd.h headers are the sanctioned way through a layering barrier: they
carry forward declarations (and lightweight aliases) only, so they cost
nothing to include and are exempt from the layering rules by suffix. When a
hub header needs a subsystem's type names but not its definitions, put the
declarations in <subsystem>_fwd.h (precedent:
exec/common/hash_table/phmap_fwd_decl.h) and include the real headers only
in .cpp files. Do not put function bodies, class bodies, or includes of
non-fwd project headers into a *_fwd.h.