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

   > Split out of **https://github.com/apache/doris/pull/66510**. That PR 
carries the
   > whole BE build-time batch and its end-to-end measurements — **please refer 
to
   > #66510 for the complete benefit numbers**. This PR only lands the 
measurement
   > tooling that batch was built on, so that the later header/instantiation PRs
   > have a gate to point at. It has no dependency on the rest and can be 
reviewed
   > and merged on its own.
   
   ### What problem does this PR solve?
   
   Related PR: #66510
   
   Problem Summary:
   
   Three measurement tools under `build-support/compile-bench/`, plus one real 
bug
   the tools found. None of them enters the build graph.
   
   #### 1. `closure-sweep.sh` + `syntax_sweep.py --no-pch` — the safety net for 
header surgery
   
   Any PR that cuts an include edge risks breaking a TU that was only compiling
   because some other header dragged the symbol in. The existing 
`syntax_sweep.py`
   could not catch that class of debt, for two reasons:
   
   - **With the PCH active, transitive-dependency debt is invisible.** Symbols 
leak
     out of `cmake_pch.hxx` into every TU, so a TU that forgot an `#include` 
still
     compiles. `--no-pch` strips the PCH preamble and runs each TU
     `-fsyntax-only` against its **natural** include closure. As a bonus it 
keeps
     the sweep usable while a header included by `pch.h` has been edited but the
     `.pch` is stale.
   - **109 compilation units were never being swept at all.** The scan filtered
     `compile_commands.json` by the `be/src` path prefix, but CMake unity 
batches
     emit their `.cxx` under the *build* directory, so every unity-covered TU 
was
     silently skipped. Including them takes the real scan surface to 191 
standalone
     TUs + 109 unity batches = 300 compilation units.
   
   `closure-sweep.sh` wraps this into a gate: it archives per-tag failure lists
   under `be/compile-bench-results/sweeps/`, records a baseline (existing 
debt), and
   in diff mode exits 1 on any *new* failure versus that baseline.
   
   #### 2. What the net caught on its first run
   
   `DeleteBitmap::diffset` in `tablet_meta.cpp` uses `std::ranges::views` but 
never
   includes `<ranges>` — the TU only compiled because the PCH leaked it in via
   simdjson. Exactly the class of debt the `--no-pch` mode exists to find, so 
the
   one-line fix ships here as the tool's first result.
   
   #### 3. `rebuild_radius.py` — how many objects does touching this header 
rebuild?
   
   Counts, per header, the object targets that list it in ninja's dependency
   database, and flags the ones reachable from `pch.h` — those rebuild every
   first-party object no matter how few TUs actually use them, which is what 
makes
   them worth cutting.
   
   The obvious alternative, `touch <header> && ninja -n | wc -l`, **cannot work 
in
   this repo**: `CONFIGURE_DEPENDS` globs stop the dry run at `Re-running CMake`
   before it lists a single compile edge. Hence the `ninja -t deps` approach.
   
   #### 4. `tu-bench.sh` — single-TU compile timing probe
   
   Companion to `--compile-bench` for before/after comparison of one translation
   unit. Looks up the TU's real command in `compile_commands.json`, strips the
   binary PCH load (keeping the textual include of `cmake_pch.hxx` so 
forced-header
   semantics stay identical), and reports wall seconds, max RSS, weak-definition
   count and total `.text` bytes, appending one TSV line per invocation. Runs on
   both Linux and macOS (`/usr/bin/time -v`, `size -A` and the ELF 
weak-definition
   nm letters are all GNU-only; the Darwin paths use the Apple equivalents).
   
   ### Release note
   
   None
   
   ### Check List (For Author)
   
   - Test
       - [x] No need to test or manual test. Explain why:
           - [x] Other reason: everything here lives under `build-support/` and 
is
             not part of the build graph — no target compiles, links or runs 
these
             scripts. The single source change, `#include <ranges>` in
             `tablet_meta.cpp`, is a missing include for code already in the 
tree
             and is covered by the existing build.
       - [x] Manual test — the tools were exercised throughout #66510: the 
300-unit
         closure sweep ran clean (300/300) against the natural-closure baseline
         after each header-surgery wave, and `tu-bench.sh` / `rebuild_radius.py`
         produced the per-TU and rebuild-radius numbers quoted there.
   
   - Behavior changed:
       - [x] No.
   
   - Does this need documentation?
       - [x] No. Each script self-documents via `--help` / a header comment.
   
   ### Proactive disclosure
   
   - All three tools were developed and run on macOS / clang 20. They are
     POSIX-shell and Python 3 with no exotic dependencies, and `tu-bench.sh`
     explicitly branches Linux vs Darwin, but the Linux paths of `tu-bench.sh` 
have
     had far less mileage than the Darwin ones.
   - `rebuild_radius.py` reads ninja's dep database, so it needs a *populated*
     build directory to be meaningful; on a fresh configure it reports nothing.
   - The sweep gate is opt-in — nothing in CI invokes it. Wiring it into CI 
would be
     a separate discussion (the full 300-unit `-fsyntax-only` pass is not free).
   


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