morningman opened a new pull request, #67036:
URL: https://github.com/apache/doris/pull/67036
### What problem does this PR solve?
Issue Number: none
Related PR: #66901 (header-hygiene gate this PR extends), #66715 (BE
build-speed umbrella, closed)
Problem Summary:
`common/status.h` is included by ~2450 of ~2500 BE TUs and carried
`gen_cpp/Status_types.h` (the whole thrift runtime) plus `gen_cpp/types.pb.h`
(the protobuf runtime) into every one of them. For any TU whose only path to
the generated headers was status.h, that is ~44k preprocessed lines of pure
tax per TU — paid in every build without a usable PCH: the Linux BE UT line
(be/test builds with no PCH), `ENABLE_PCH=OFF` (gcc) builds,
clangd/clang-tidy,
and every incremental build after touching `Status.thrift`/`types.proto`.
This PR decouples status.h/exception.h (and `util/hash_util.hpp`) from the
generated headers, with **zero runtime semantic change**:
1. **status.h**: `APPLY_FOR_THRIFT_ERROR_CODES` now carries the literal enum
value per entry; `status.cpp` locks every value to the thrift enum with a
`static_assert`, so `Status.thrift` stays the single source of truth — any
drift is a compile error. `Status::create<stacktrace>(TStatus/PStatus)`
move out of line with 4 explicit instantiations (cold path: RPC error
conversion), `code_as_string()` moves out of line (cold path: error
printing), and both generated includes are dropped (`TStatus` joins
`PStatus` as a forward declaration).
2. **exception.h**: drops its unused `Status_types.h` include.
3. **hash_util.hpp**: the `std::hash<TUniqueId/TNetworkAddress/pair>`
specializations keep only their declarations here (this header stays the
earliest carrier, so they remain visible before any implicit
instantiation); the bodies move to `uid_util.cpp`/`network_util.cpp` where
the types are complete. The `Types_types.h` include is then dropped.
4. Mechanical fallout, all preparation-wave commits: `endian.h` gets its own
`gbswap_16/32` builtins instead of leaning on the `bswap_*` macros that
protobuf's `stubs/port.h` happened to leak into most TUs;
`IDataType::to_protobuf(PTypeDesc*)` and the nine derived overrides move
out of line (data_type.h forward-declares the pb types); every TU that
named `TStatusCode`/`TStatus`/`PStatus`/`PValues` or `std::set`/
`std::unordered_set` via ride-along gets its direct include.
**Numbers** (clang -E, macOS arm; text include-graph reach over
be/src+be/test):
| metric | before | after |
|---|---|---|
| `common/status.h` closure | 147,838 lines | **103,883 (-29.7%)** |
| `util/hash_util.hpp` closure | 178,644 lines | **131,801 (-26.2%)** |
| `util/bitmap.cpp` natural closure (representative winner) | 172,721 |
**128,752 (-25%)** |
| `gen_cpp/Status_types.h` reach | 2453 TUs | **37 TUs** |
| `gen_cpp/types.pb.h` reach | 2458 TUs | 1791 TUs |
Cold build wall-clock with clang + `ENABLE_PCH=ON` is **neutral** (687.3s ->
693.6s, +0.9%, paired back-to-back `--compile-bench -j 6` runs, within
run-to-run noise) — expected, since pch.h itself still includes the generated
headers. The wins are the no-PCH worlds and the incremental radius: editing
`Status.thrift` no longer invalidates essentially every TU there.
**Guards** (extends #66901): `check-header-deps.py` now forbids
status.h/exception.h/hash_util.hpp from reaching `gen_cpp/*` (empty
whitelists), the exec_env gen_cpp whitelist empties out, thread_context loses
its `Status_types.h` entry, and `REVERSE_REACH_BASELINES` pins the three
generated headers at their new reach so the tax cannot silently regrow.
`syntax_sweep.py` gains `--include-tests` so the UT line's natural closures
are sweepable.
### Release note
None
### Check List (For Author)
- Test <!-- At least one of them must be included. -->
- [ ] Regression test
- [x] Unit Test
- [x] Manual test (add detailed scripts or steps below)
- `-fsyntax-only` natural-closure sweeps (PCH stripped) at three
scopes, all zero regressions vs pre-surgery baselines: Release
unity scope (315 TUs), UT test scope with `--include-tests`
(1167 be/test TUs), and a per-file sweep of all 1497 be/src
sources (defeats unity masking);
- full `build.sh --be` compile + `doris_be` link green;
`status_test.cpp` / `exception_test.cpp` compiled to .o with the
UT flag set;
- guard self-test: re-injecting `#include <gen_cpp/types.pb.h>`
into status.h makes `check-header-deps.py` fail as intended;
- paired cold-build bench, see numbers above.
- [ ] No need to test or manual test. Explain why:
- [ ] This is a refactor/code format and no logic has been changed.
- [ ] Previous test can cover this change.
- [ ] No code files have been changed.
- [ ] Other reason <!-- Add your reason? -->
- Behavior changed:
- [x] No.
- [ ] Yes. <!-- Explain the behavior change -->
- Does this need documentation?
- [x] No.
- [ ] Yes. <!-- Add document PR link here. eg:
https://github.com/apache/doris-website/pull/1214 -->
### Check List (For Reviewer who merge this PR)
- [ ] Confirm the release note
- [ ] Confirm test cases
- [ ] Confirm document
- [ ] Add branch pick label <!-- Add branch pick label that this PR should
merge into -->
🤖 Generated with [Claude Code](https://claude.com/claude-code)
https://claude.ai/code/session_0167LexPzAjQuu5yHvaH9hmo
--
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]
