github-actions[bot] commented on code in PR #66400: URL: https://github.com/apache/doris/pull/66400#discussion_r3712044164
########## build-support/compile-bench/syntax_sweep.py: ########## @@ -0,0 +1,124 @@ +#!/usr/bin/env python3 +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +"""Parallel -fsyntax-only sweep over BE TUs using compile_commands.json. + +Validates include-structure changes (header cuts, forward-declaration swaps) +against every TU without mutating the ninja build state: each compile command +is replayed with -o/-c/-MD/-MT/-MF/-ftime-trace* stripped and -fsyntax-only +appended, so nothing is written to the build directory. Front-end-only checks +run in roughly half the time of a real compile and catch every missing-include +or missing-declaration fallout a cut can cause. + +Usage: + python3 syntax_sweep.py [--build-dir DIR] [--jobs N] [--filter SUBSTR] + [--fail-log FILE] + + --filter limits the sweep to TUs whose source path contains SUBSTR + (e.g. --filter load/memtable for a quick re-check of one subsystem). + +Exit code: 0 if every TU passes, 1 otherwise. +""" + +import argparse +import concurrent.futures +import json +import os +import re +import shlex +import subprocess +import sys +import time + +SCRIPT_DIR = os.path.dirname(os.path.abspath(__file__)) +REPO_ROOT = os.path.dirname(os.path.dirname(SCRIPT_DIR)) +ANSI = re.compile(r"\x1b\[[0-9;]*m") + +STRIP_WITH_ARG = ("-o", "-MT", "-MF") +STRIP_FLAGS = ("-c", "-MD", "-MMD") + + +def mangle(cmd): + args = shlex.split(cmd) + out, skip = [], False + for a in args: + if skip: + skip = False + continue + if a in STRIP_WITH_ARG: + skip = True + continue + if a in STRIP_FLAGS or a.startswith("-ftime-trace"): + continue + out.append(a) + out.append("-fsyntax-only") + return out + + +def main(): + ap = argparse.ArgumentParser(description=__doc__, + formatter_class=argparse.RawDescriptionHelpFormatter) + ap.add_argument("--build-dir", + default=os.path.join(REPO_ROOT, "be", "build_Release_compile_bench")) + ap.add_argument("--jobs", type=int, default=max(2, (os.cpu_count() or 8) // 2)) + ap.add_argument("--filter", default="", + help="only sweep TUs whose path contains this substring") + ap.add_argument("--fail-log", default="", + 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 Review 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. ########## build.sh: ########## @@ -657,6 +674,22 @@ for ((i = 0; i < ${#CLOUD_EXTRA_FEATURE_KEYS[@]}; i++)); do CLOUD_EXTRA_CMAKE_ARGS+=("-D${feature_name}_MODULE_DIR=${CLOUD_EXTRA_MODULE_PATHS[i]}") done +if [[ "${COMPILE_BENCH}" -eq 1 ]]; then Review 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. ########## build-support/check-header-deps.py: ########## @@ -61,6 +65,138 @@ "them; reaching the index implementation headers from here puts the whole " "index writer stack (and CLucene) in front of most of the backend", ), + ( + "runtime/exec_env.h", Review 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. ########## build.sh: ########## @@ -657,6 +674,22 @@ for ((i = 0; i < ${#CLOUD_EXTRA_FEATURE_KEYS[@]}; i++)); do CLOUD_EXTRA_CMAKE_ARGS+=("-D${feature_name}_MODULE_DIR=${CLOUD_EXTRA_MODULE_PATHS[i]}") done +if [[ "${COMPILE_BENCH}" -eq 1 ]]; then + # BE compile benchmark mode: measure a cold, cache-free BE C++ build. + # Everything that is not the BE C++ build would only add noise, so force + # a BE-only build regardless of the other options. + BUILD_BE=1 + BUILD_FE=0 + BUILD_CLOUD=0 + BUILD_HIVE_UDF=0 + BUILD_BE_JAVA_EXTENSIONS=0 + BUILD_BE_CDC_CLIENT=0 + OUTPUT_BE_BINARY=0 Review 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. ########## build-support/compile-bench/cut_impact.py: ########## @@ -0,0 +1,489 @@ +#!/usr/bin/env python3 +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +"""Estimate the blast radius of cutting one #include edge from a hub header. + +Before removing `#include "T"` from hub header H, you want to know: + 1. which TUs currently reach T only through H (they will stop seeing T); + 2. which of those TUs (or headers they keep) actually reference symbols + declared in T's include subtree, and therefore need a direct include + added ("seeding") before the edge can be cut safely. + +Data sources: + * `ninja -t deps` from a completed compile-bench build directory: the real, + per-TU flat header closure (what each TU actually included in this build + configuration). + * The parsed include graph of be/src + gensrc/build: edge structure, built + by scanning `#include` directives and resolving them the way the compiler + does (includer dir first for quoted includes, then -I roots). + +Per candidate TU the script runs two BFS traversals over the parsed graph, +both restricted to the TU's real closure: one with the edge, one without. +The difference is the set of headers this TU loses. Symbols defined in lost +headers are then matched (word-level, comments stripped) against the files +the TU keeps; every hit becomes a seeding suggestion "file F must directly +include header L". The result is an estimate — conditional includes the +parser cannot see and symbol matches inside string literals can produce +noise — so spot-check a sample (e.g. -fsyntax-only) before mass edits. + +Usage: + # single edge: what happens if H stops including T + python3 cut_impact.py edge runtime/exec_env.h \ + information_schema/schema_routine_load_job_scanner.h + + # rank every direct project include of a hub by cut impact + python3 cut_impact.py audit runtime/exec_env.h + + # machine-readable detail / manual-verification sample + python3 cut_impact.py edge H T --json out.json --sample 5 +""" + +import argparse +import collections +import json +import os +import random +import re +import subprocess +import sys + +SCRIPT_DIR = os.path.dirname(os.path.abspath(__file__)) +REPO_ROOT = os.path.dirname(os.path.dirname(SCRIPT_DIR)) + +SOURCE_EXTS = (".cpp", ".cc", ".c", ".cxx") +HEADER_EXTS = (".h", ".hpp", ".hh", ".inc", ".ipp") + +INCLUDE_RE = re.compile(r'^\s*#\s*include\s+(["<])([^">]+)[">]', re.M) +COMMENT_RE = re.compile(r"//[^\n]*|/\*.*?\*/", re.S) +TOKEN_RE = re.compile(r"[A-Za-z_]\w{2,}") +# Type *definitions* only (a trailing '{' is required), not forward decls. +TYPE_DEF_RE = re.compile( + r"\b(?:class|struct|enum(?:\s+(?:class|struct))?)\s+" + r"(?:\[\[[^\]]*\]\]\s*|[A-Z_]{3,}\s+)?" # attributes / export macros + r"([A-Za-z_]\w*)\s*(?:final\s*)?(?::[^;{}]*)?\{" +) +USING_RE = re.compile(r"\busing\s+([A-Za-z_]\w*)\s*=") +TYPEDEF_RE = re.compile(r"\btypedef\b[^;]*?\b([A-Za-z_]\w*)\s*;") +DEFINE_RE = re.compile(r"^\s*#\s*define\s+([A-Za-z_]\w*)", re.M) + + +def read_text(path): + try: + with open(path, "r", encoding="utf-8", errors="replace") as f: + return f.read() + except OSError: + return "" + + +class IncludeGraph: + """Parsed include-edge structure over project files (be/src, gensrc/build).""" + + def __init__(self, roots): + self.roots = roots # ordered list of abs dirs, first match wins for display + self.files = {} # abs path -> True + self.includes = {} # abs path -> list of abs paths (project files only) + self._stripped = {} # abs path -> comment-stripped text (lazy) + self._tokens = {} # abs path -> set of identifier tokens (lazy) + self._symbols = {} # abs path -> set of defined top-level names (lazy) + self._scan() + self._parse_edges() + + def _scan(self): + for root in self.roots: + for dirpath, _dirnames, filenames in os.walk(root): + for name in filenames: + if name.endswith(SOURCE_EXTS) or name.endswith(HEADER_EXTS): + p = sys.intern(os.path.normpath(os.path.join(dirpath, name))) + self.files[p] = True + + def _parse_edges(self): + for path in self.files: + edges = [] + for m in INCLUDE_RE.finditer(read_text(path)): + quoted = m.group(1) == '"' + target = self.resolve(m.group(2), os.path.dirname(path), quoted) + if target is not None and target != path: + edges.append(target) + self.includes[path] = edges + + def resolve(self, inc, includer_dir, quoted=True): + if quoted: + cand = sys.intern(os.path.normpath(os.path.join(includer_dir, inc))) + if cand in self.files: + return cand + for root in self.roots: + cand = sys.intern(os.path.normpath(os.path.join(root, inc))) + if cand in self.files: + return cand + return None + + def display(self, path): + for root in self.roots: + if path.startswith(root + os.sep): + return os.path.relpath(path, root) + return path + + def subtree(self, start): + """All files reachable from `start` in the unrestricted parsed graph.""" + seen = {start} + queue = collections.deque([start]) + while queue: + for nxt in self.includes.get(queue.popleft(), ()): + if nxt not in seen: + seen.add(nxt) + queue.append(nxt) + return seen + + def stripped_text(self, path): + if path not in self._stripped: + self._stripped[path] = COMMENT_RE.sub(" ", read_text(path)) + return self._stripped[path] + + def tokens(self, path): + if path not in self._tokens: + self._tokens[path] = set(TOKEN_RE.findall(self.stripped_text(path))) + return self._tokens[path] + + def symbols(self, path): + """Top-level names *defined* by this header (types, aliases, macros).""" + if path not in self._symbols: + text = self.stripped_text(path) + names = set() + for regex in (TYPE_DEF_RE, USING_RE, TYPEDEF_RE, DEFINE_RE): + names.update(regex.findall(text)) + self._symbols[path] = {n for n in names if len(n) >= 3} + return self._symbols[path] + + +def load_tus(build_dir, graph): + """source abs path -> set of project files in its real (ninja) closure.""" + proc = subprocess.Popen( + ["ninja", "-C", build_dir, "-t", "deps"], + stdout=subprocess.PIPE, + stderr=subprocess.DEVNULL, + text=True, + ) + tus = {} + src_prefix = os.path.join(REPO_ROOT, "be", "src") + os.sep + gen_prefix = os.path.join(REPO_ROOT, "gensrc", "build") + os.sep + cur = None # dep set of the current block's TU; None while skipping a block + expect_source = False # the next indented line is the block's source file + for line in proc.stdout: + if line.startswith(" "): + p = line.strip() + if not os.path.isabs(p): + p = os.path.join(build_dir, p) + p = os.path.normpath(p) + if expect_source: # first dep line = the source file itself + expect_source = False + if p.startswith((src_prefix, gen_prefix)): + key = sys.intern(p) + cur = tus.setdefault(key, set()) + cur.add(key) + else: + cur = None # foreign TU (contrib etc.): skip whole block + elif cur is not None and p in graph.files: + cur.add(sys.intern(p)) + else: # block header "<target>: #deps N, ... (VALID|STALE)" or noise + expect_source = line.rstrip().endswith("(VALID)") + cur = None + proc.wait() Review 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. ########## build-support/compile-bench/report.py: ########## @@ -0,0 +1,533 @@ +#!/usr/bin/env python3 +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +"""Report generator for `build.sh --compile-bench` runs. + +Generate a report for one run (done automatically at the end of a bench build, +can be re-run manually at any time): + + python3 report.py <run_dir> [--build-dir DIR] [--top N] [--build-status S] + +Compare two runs (arguments are run dirs or summary.json paths): + + python3 report.py compare <old> <new> + +Inputs inside <run_dir> (see bench-lib.sh): + meta.tsv, phases.tsv, compile_log.jsonl, ninja_log.txt +Optional, from the build dir when COMPILE_BENCH_TRACE=ON was used: + per-TU clang -ftime-trace JSON files (next to the object files) + +Outputs inside <run_dir>: report.txt (human) and summary.json (machine). +""" + +import argparse +import json +import os +import sys +from collections import defaultdict + +TOP_DIRS = 25 +TOP_HEADERS = 30 +TOP_TEMPLATES = 20 +TOP_TAIL = 10 +WIDTH = 78 + + +def section(title): + text = "-- " + title + " " + return text + "-" * max(0, WIDTH - len(text)) + + +def fmt_dur(seconds): + seconds = float(seconds) + if seconds < 0: + return "?" + if seconds < 60: + return "{:.1f}s".format(seconds) + minutes = int(seconds // 60) + if minutes < 60: + return "{}m{:02d}s".format(minutes, int(seconds % 60)) + return "{}h{:02d}m".format(minutes // 60, minutes % 60) + + +def read_meta(run_dir): + meta = {} + path = os.path.join(run_dir, "meta.tsv") + if os.path.isfile(path): + with open(path) as fh: + for line in fh: + parts = line.rstrip("\n").split("\t", 1) + if len(parts) == 2: + meta[parts[0]] = parts[1] + return meta + + +def read_phases(run_dir): + phases = [] + path = os.path.join(run_dir, "phases.tsv") + if os.path.isfile(path): + with open(path) as fh: + for line in fh: + parts = line.rstrip("\n").split("\t") + if len(parts) != 3: + continue + try: + start_ms, end_ms = int(parts[1]), int(parts[2]) + except ValueError: + continue + phases.append( + {"name": parts[0], "dur_s": (end_ms - start_ms) / 1000.0} + ) + return phases + + +def is_cmake_probe(record): + for key in ("src", "out", "cwd"): + value = record.get(key) or "" + if "CMakeScratch" in value or "CMakeTmp" in value: + return True + return False + + +def read_compile_log(run_dir): + records = [] + path = os.path.join(run_dir, "compile_log.jsonl") + if os.path.isfile(path): + with open(path) as fh: + for line in fh: + line = line.strip() + if not line: + continue + try: + record = json.loads(line) + except ValueError: + continue + if not is_cmake_probe(record): + records.append(record) + return records + + +def read_ninja_log(run_dir, build_dir): + """Return {output_path: (start_ms, end_ms)}, deduped keeping the last entry.""" + path = os.path.join(run_dir, "ninja_log.txt") + if not os.path.isfile(path) and build_dir: + path = os.path.join(build_dir, ".ninja_log") + edges = {} + if not os.path.isfile(path): + return edges + with open(path) as fh: + for line in fh: + if line.startswith("#"): + continue + parts = line.rstrip("\n").split("\t") + if len(parts) < 4: + continue + try: + start_ms, end_ms = int(parts[0]), int(parts[1]) + except ValueError: + continue + edges[parts[3]] = (start_ms, end_ms) + return edges + + +def rel_path(path, meta): + if not path: + return "?" + doris_home = meta.get("doris_home") + if doris_home: + home = doris_home.rstrip("/") + "/" + if path.startswith(home): + return path[len(home):] + return path + + +def shorten_header(path, meta): + doris_home = (meta.get("doris_home") or "").rstrip("/") + if doris_home: + if path.startswith(doris_home + "/thirdparty/installed/include/"): + return "<thirdparty>/" + path[len(doris_home + "/thirdparty/installed/include/"):] + if path.startswith(doris_home + "/"): + return path[len(doris_home) + 1:] + return path + + +def group_keys(rel_src): + """Return (level1, level2) directory grouping keys for a doris_home-relative source.""" + rel_dir = os.path.dirname(rel_src) + if rel_dir.startswith("be/src"): + prefix, rest = "be/src", rel_dir[len("be/src"):].strip("/") + else: + prefix, rest = "", rel_dir + parts = [p for p in rest.split("/") if p] + level1 = "/".join([prefix] + parts[:1]) if prefix else ("/".join(parts[:1]) or ".") + level2 = "/".join([prefix] + parts[:2]) if prefix else ("/".join(parts[:2]) or ".") + return level1 or ".", level2 or "." + + +def load_time_traces(compiles, meta, build_dir): + """Aggregate clang -ftime-trace JSONs written next to the object files. + + Durations reported by clang are microseconds. "Source" and template + instantiation timings are inclusive of nested work, so sums across headers + overlap; they rank hotspots, they are not additive wall time. + """ + headers = defaultdict(lambda: [0.0, 0]) # path -> [total_s, count] + templates = defaultdict(lambda: [0.0, 0]) # symbol -> [total_s, count] + tu_split = {} # src -> {total, frontend, backend} + parsed = 0 + for record in compiles: + out = record.get("out") + if not out: + continue + base = os.path.join(record.get("cwd") or build_dir or "", out) + trace_path = os.path.splitext(base)[0] + ".json" + if not os.path.isfile(trace_path): + continue + try: + with open(trace_path) as fh: + events = json.load(fh).get("traceEvents", []) + except (ValueError, OSError): + continue + parsed += 1 + maxima = defaultdict(float) + # clang >= 20 emits "Source" as async begin/end pairs (ph "b"/"e") that + # nest on one tid; older clang emits complete events with "dur". + source_stacks = defaultdict(list) + for event in events: + dur_s = event.get("dur", 0) / 1e6 + name = event.get("name", "") + detail = (event.get("args") or {}).get("detail", "") + if name == "Source": + phase = event.get("ph") + if phase == "b": + source_stacks[event.get("tid")].append((detail, event.get("ts", 0))) + continue + if phase == "e": + stack = source_stacks.get(event.get("tid")) + if not stack: + continue + detail, begin_ts = stack.pop() + dur_s = (event.get("ts", 0) - begin_ts) / 1e6 + if detail: + entry = headers[detail] + entry[0] += dur_s + entry[1] += 1 + elif name in ("InstantiateClass", "InstantiateFunction") and detail: + entry = templates[detail] + entry[0] += dur_s + entry[1] += 1 + elif name in ("ExecuteCompiler", "Frontend", "Backend", + "Total Frontend", "Total Backend"): + if dur_s > maxima[name]: + maxima[name] = dur_s + tu_split[record.get("src") or out] = { + "total_s": maxima["ExecuteCompiler"], + "frontend_s": max(maxima["Frontend"], maxima["Total Frontend"]), + "backend_s": max(maxima["Backend"], maxima["Total Backend"]), + } + return { + "parsed": parsed, + "headers": headers, + "templates": templates, + "tu_split": tu_split, + } + + +def build_report(run_dir, build_dir, top_n, build_status): + meta = read_meta(run_dir) + if not build_dir: + build_dir = meta.get("build_dir") + phases = read_phases(run_dir) + records = read_compile_log(run_dir) + ninja_edges = read_ninja_log(run_dir, build_dir) + + compiles = [r for r in records if r.get("kind") in ("compile", "pch")] + links = [r for r in records if r.get("kind") == "link"] + failed = [r for r in records if r.get("rc", 0) != 0] + + lines = [] + out = lines.append + out("=" * 78) + out(" Doris BE compile benchmark report run: {}".format( + meta.get("run_id", os.path.basename(run_dir.rstrip("/"))))) + out("=" * 78) + out(" build status : {}".format(build_status)) + for key in ("date_utc", "git_branch", "git_commit", "uname", "ncpu", "parallel", + "build_type", "generator", "toolchain", "cxx_version", "enable_pch", + "time_trace", "build_dir"): + if key in meta: + out(" {:<13}: {}".format(key, meta[key])) + + # ---- Phases ------------------------------------------------------------- + out("") + out(section("Phases")) + total_s = None + for phase in phases: + if phase["name"] == "total": + total_s = phase["dur_s"] + for phase in phases: + if phase["name"] == "total": + continue + pct = " ({:5.1f}%)".format(100.0 * phase["dur_s"] / total_s) if total_s else "" + out(" {:<24} {:>8}{}".format(phase["name"], fmt_dur(phase["dur_s"]), pct)) + if total_s is not None: + out(" {:<24} {:>8}".format("total (wall)", fmt_dur(total_s))) + + build_phase_s = None + for phase in phases: + if phase["name"] == "build": + build_phase_s = phase["dur_s"] + + # ---- Build summary ------------------------------------------------------ + out("") + out(section("Build summary")) + sum_wall = sum(r["wall_s"] for r in compiles) + sum_cpu = sum(r["user_s"] + r["sys_s"] for r in compiles) + out(" compile units (compile+pch) : {}".format(len(compiles))) + out(" sum of TU wall time : {}".format(fmt_dur(sum_wall))) + out(" sum of TU cpu time (user+sys) : {}".format(fmt_dur(sum_cpu))) + if build_phase_s: + out(" build phase wall : {}".format(fmt_dur(build_phase_s))) + edge_sum = sum_wall + sum(r["wall_s"] for r in links) + out(" effective parallelism : {:.1f}x (sum TU+link wall / build wall)" + .format(edge_sum / build_phase_s)) + if compiles: + slowest = max(compiles, key=lambda r: r["wall_s"]) + out(" slowest single TU : {} ({})".format( + fmt_dur(slowest["wall_s"]), rel_path(slowest.get("src"), meta))) + hungriest = max(compiles, key=lambda r: r.get("maxrss_mb", 0)) + out(" largest TU peak rss : {:.0f} MB ({})".format( + hungriest.get("maxrss_mb", 0), rel_path(hungriest.get("src"), meta))) + for link in sorted(links, key=lambda r: r["wall_s"], reverse=True)[:5]: + out(" link {:<24} : {} peak rss {:.0f} MB".format( + os.path.basename(link.get("out") or "?"), + fmt_dur(link["wall_s"]), link.get("maxrss_mb", 0))) + if failed: + out(" FAILED commands : {}".format(len(failed))) + for record in failed[:10]: + out(" rc={:<4} {}".format( + record.get("rc"), rel_path(record.get("src") or record.get("out"), meta))) + + # ---- Top slow TUs ------------------------------------------------------- + out("") + out(section("Top {} slowest translation units (wall)".format(top_n))) + out(" {:>4} {:>8} {:>8} {:>7} {:>9} {}".format( + "rank", "wall", "user", "sys", "maxrss", "file")) + ranked = sorted(compiles, key=lambda r: r["wall_s"], reverse=True) + for idx, record in enumerate(ranked[:top_n], 1): + out(" {:>4} {:>8} {:>8} {:>7} {:>7.0f}MB {}{}".format( + idx, fmt_dur(record["wall_s"]), fmt_dur(record["user_s"]), + fmt_dur(record["sys_s"]), record.get("maxrss_mb", 0), + rel_path(record.get("src"), meta), + " [pch]" if record.get("kind") == "pch" else "")) + + # ---- Directory rollup --------------------------------------------------- + for level, title in ((0, "top-level directory"), (1, "second-level directory")): + rollup = defaultdict(lambda: [0.0, 0]) + for record in compiles: + rel = rel_path(record.get("src") or record.get("out") or "?", meta) + key = group_keys(rel)[level] + entry = rollup[key] + entry[0] += record["wall_s"] + entry[1] += 1 + out("") + out(section("Wall time by {}".format(title))) + out(" {:>9} {:>6} {:>8} {}".format("wall-sum", "count", "avg", "directory")) + ordered = sorted(rollup.items(), key=lambda kv: kv[1][0], reverse=True) + for key, (wall, count) in ordered[:TOP_DIRS]: + out(" {:>9} {:>6} {:>8} {}".format( + fmt_dur(wall), count, fmt_dur(wall / count), key)) + + # ---- Ninja tail: what the build waits on at the end --------------------- + if ninja_edges: + out("") + out(section("Last finishers (critical-path tail, from .ninja_log)")) + out(" {:>10} {:>10} {:>8} {}".format("start", "end", "dur", "output")) + tail = sorted(ninja_edges.items(), key=lambda kv: kv[1][1], reverse=True) + for output, (start_ms, end_ms) in tail[:TOP_TAIL]: + out(" {:>10} {:>10} {:>8} {}".format( + fmt_dur(start_ms / 1000.0), fmt_dur(end_ms / 1000.0), + fmt_dur((end_ms - start_ms) / 1000.0), output)) + + # ---- Optional -ftime-trace analysis ------------------------------------- + trace = None + if meta.get("time_trace") == "ON" and build_dir: + trace = load_time_traces(compiles, meta, build_dir) + out("") + out(section("[-ftime-trace] parsed {} trace files".format(trace["parsed"]))) + if trace["parsed"]: + out("") + out(" Top headers by inclusive parse time (overlapping, ranks hotspots):") + out(" {:>9} {:>7} {:>8} {}".format("total", "count", "avg", "header")) + for path, (total, count) in sorted( + trace["headers"].items(), key=lambda kv: kv[1][0], + reverse=True)[:TOP_HEADERS]: + out(" {:>9} {:>7} {:>8} {}".format( + fmt_dur(total), count, fmt_dur(total / count), + shorten_header(path, meta))) + out("") + out(" Top template instantiations (inclusive):") + out(" {:>9} {:>7} {}".format("total", "count", "symbol")) + for symbol, (total, count) in sorted( + trace["templates"].items(), key=lambda kv: kv[1][0], + reverse=True)[:TOP_TEMPLATES]: + out(" {:>9} {:>7} {}".format(fmt_dur(total), count, symbol[:110])) + out("") + out(" Frontend (parse/instantiate) vs backend (codegen/opt) of slowest TUs:") + out(" {:>9} {:>9} {:>9} {}".format("total", "frontend", "backend", "file")) + for record in ranked[:15]: + split = trace["tu_split"].get(record.get("src") or "") + if not split: + continue + out(" {:>9} {:>9} {:>9} {}".format( + fmt_dur(split["total_s"]), fmt_dur(split["frontend_s"]), + fmt_dur(split["backend_s"]), rel_path(record.get("src"), meta))) + else: + out(" (no trace files found under {} - was the build dir wiped?)" + .format(build_dir)) + + out("") + out("=" * 78) + + summary = { + "meta": meta, + "build_status": build_status, + "phases": {p["name"]: round(p["dur_s"], 1) for p in phases}, + "totals": { + "compile_units": len(compiles), + "sum_tu_wall_s": round(sum_wall, 1), + "sum_tu_cpu_s": round(sum_cpu, 1), + "build_phase_s": round(build_phase_s, 1) if build_phase_s else None, + }, + "files": { + rel_path(r.get("src") or r.get("out"), meta): { + "wall_s": r["wall_s"], + "user_s": r["user_s"], + "maxrss_mb": r.get("maxrss_mb", 0), + "kind": r.get("kind"), + } + for r in compiles + links + }, + } + if trace and trace["parsed"]: + summary["headers_top"] = { + shorten_header(path, meta): round(total, 1) + for path, (total, _) in sorted( + trace["headers"].items(), key=lambda kv: kv[1][0], reverse=True)[:100] + } + summary["templates_top"] = { + symbol[:200]: round(total, 1) + for symbol, (total, _) in sorted( + trace["templates"].items(), key=lambda kv: kv[1][0], reverse=True)[:100] + } + return lines, summary + + +def cmd_report(args): + run_dir = args.run_dir + if not os.path.isdir(run_dir): + print("ERROR: run dir not found: {}".format(run_dir), file=sys.stderr) + return 1 + lines, summary = build_report(run_dir, args.build_dir, args.top, args.build_status) + report_path = os.path.join(run_dir, "report.txt") + with open(report_path, "w") as fh: + fh.write("\n".join(lines) + "\n") + with open(os.path.join(run_dir, "summary.json"), "w") as fh: + json.dump(summary, fh, indent=1, sort_keys=True) + print("\n".join(lines)) + print("Report written to {}".format(report_path)) + return 0 + + +def load_summary(path): + if os.path.isdir(path): + path = os.path.join(path, "summary.json") + with open(path) as fh: + return json.load(fh) + + +def cmd_compare(args): Review 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-support/compile-bench/report.py: ########## @@ -0,0 +1,533 @@ +#!/usr/bin/env python3 +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +"""Report generator for `build.sh --compile-bench` runs. + +Generate a report for one run (done automatically at the end of a bench build, +can be re-run manually at any time): + + python3 report.py <run_dir> [--build-dir DIR] [--top N] [--build-status S] + +Compare two runs (arguments are run dirs or summary.json paths): + + python3 report.py compare <old> <new> + +Inputs inside <run_dir> (see bench-lib.sh): + meta.tsv, phases.tsv, compile_log.jsonl, ninja_log.txt +Optional, from the build dir when COMPILE_BENCH_TRACE=ON was used: + per-TU clang -ftime-trace JSON files (next to the object files) + +Outputs inside <run_dir>: report.txt (human) and summary.json (machine). +""" + +import argparse +import json +import os +import sys +from collections import defaultdict + +TOP_DIRS = 25 +TOP_HEADERS = 30 +TOP_TEMPLATES = 20 +TOP_TAIL = 10 +WIDTH = 78 + + +def section(title): + text = "-- " + title + " " + return text + "-" * max(0, WIDTH - len(text)) + + +def fmt_dur(seconds): + seconds = float(seconds) + if seconds < 0: + return "?" + if seconds < 60: + return "{:.1f}s".format(seconds) + minutes = int(seconds // 60) + if minutes < 60: + return "{}m{:02d}s".format(minutes, int(seconds % 60)) + return "{}h{:02d}m".format(minutes // 60, minutes % 60) + + +def read_meta(run_dir): + meta = {} + path = os.path.join(run_dir, "meta.tsv") + if os.path.isfile(path): + with open(path) as fh: + for line in fh: + parts = line.rstrip("\n").split("\t", 1) + if len(parts) == 2: + meta[parts[0]] = parts[1] + return meta + + +def read_phases(run_dir): + phases = [] + path = os.path.join(run_dir, "phases.tsv") + if os.path.isfile(path): + with open(path) as fh: + for line in fh: + parts = line.rstrip("\n").split("\t") + if len(parts) != 3: + continue + try: + start_ms, end_ms = int(parts[1]), int(parts[2]) + except ValueError: + continue + phases.append( + {"name": parts[0], "dur_s": (end_ms - start_ms) / 1000.0} + ) + return phases + + +def is_cmake_probe(record): + for key in ("src", "out", "cwd"): + value = record.get(key) or "" + if "CMakeScratch" in value or "CMakeTmp" in value: + return True + return False + + +def read_compile_log(run_dir): + records = [] + path = os.path.join(run_dir, "compile_log.jsonl") + if os.path.isfile(path): + with open(path) as fh: + for line in fh: + line = line.strip() + if not line: + continue + try: + record = json.loads(line) + except ValueError: + continue + if not is_cmake_probe(record): + records.append(record) + return records + + +def read_ninja_log(run_dir, build_dir): + """Return {output_path: (start_ms, end_ms)}, deduped keeping the last entry.""" + path = os.path.join(run_dir, "ninja_log.txt") + if not os.path.isfile(path) and build_dir: + path = os.path.join(build_dir, ".ninja_log") + edges = {} + if not os.path.isfile(path): + return edges + with open(path) as fh: + for line in fh: + if line.startswith("#"): + continue + parts = line.rstrip("\n").split("\t") + if len(parts) < 4: + continue + try: + start_ms, end_ms = int(parts[0]), int(parts[1]) + except ValueError: + continue + edges[parts[3]] = (start_ms, end_ms) + return edges + + +def rel_path(path, meta): + if not path: + return "?" + doris_home = meta.get("doris_home") + if doris_home: + home = doris_home.rstrip("/") + "/" + if path.startswith(home): + return path[len(home):] + return path + + +def shorten_header(path, meta): + doris_home = (meta.get("doris_home") or "").rstrip("/") + if doris_home: + if path.startswith(doris_home + "/thirdparty/installed/include/"): + return "<thirdparty>/" + path[len(doris_home + "/thirdparty/installed/include/"):] + if path.startswith(doris_home + "/"): + return path[len(doris_home) + 1:] + return path + + +def group_keys(rel_src): + """Return (level1, level2) directory grouping keys for a doris_home-relative source.""" + rel_dir = os.path.dirname(rel_src) + if rel_dir.startswith("be/src"): + prefix, rest = "be/src", rel_dir[len("be/src"):].strip("/") + else: + prefix, rest = "", rel_dir + parts = [p for p in rest.split("/") if p] + level1 = "/".join([prefix] + parts[:1]) if prefix else ("/".join(parts[:1]) or ".") + level2 = "/".join([prefix] + parts[:2]) if prefix else ("/".join(parts[:2]) or ".") + return level1 or ".", level2 or "." + + +def load_time_traces(compiles, meta, build_dir): + """Aggregate clang -ftime-trace JSONs written next to the object files. + + Durations reported by clang are microseconds. "Source" and template + instantiation timings are inclusive of nested work, so sums across headers + overlap; they rank hotspots, they are not additive wall time. + """ + headers = defaultdict(lambda: [0.0, 0]) # path -> [total_s, count] + templates = defaultdict(lambda: [0.0, 0]) # symbol -> [total_s, count] + tu_split = {} # src -> {total, frontend, backend} + parsed = 0 + for record in compiles: + out = record.get("out") + if not out: + continue + base = os.path.join(record.get("cwd") or build_dir or "", out) + trace_path = os.path.splitext(base)[0] + ".json" + if not os.path.isfile(trace_path): + continue + try: + with open(trace_path) as fh: + events = json.load(fh).get("traceEvents", []) + except (ValueError, OSError): + continue + parsed += 1 + maxima = defaultdict(float) + # clang >= 20 emits "Source" as async begin/end pairs (ph "b"/"e") that + # nest on one tid; older clang emits complete events with "dur". + source_stacks = defaultdict(list) + for event in events: + dur_s = event.get("dur", 0) / 1e6 + name = event.get("name", "") + detail = (event.get("args") or {}).get("detail", "") + if name == "Source": + phase = event.get("ph") + if phase == "b": + source_stacks[event.get("tid")].append((detail, event.get("ts", 0))) + continue + if phase == "e": + stack = source_stacks.get(event.get("tid")) + if not stack: + continue + detail, begin_ts = stack.pop() + dur_s = (event.get("ts", 0) - begin_ts) / 1e6 + if detail: + entry = headers[detail] + entry[0] += dur_s + entry[1] += 1 + elif name in ("InstantiateClass", "InstantiateFunction") and detail: + entry = templates[detail] + entry[0] += dur_s + entry[1] += 1 + elif name in ("ExecuteCompiler", "Frontend", "Backend", + "Total Frontend", "Total Backend"): + if dur_s > maxima[name]: + maxima[name] = dur_s + tu_split[record.get("src") or out] = { + "total_s": maxima["ExecuteCompiler"], + "frontend_s": max(maxima["Frontend"], maxima["Total Frontend"]), + "backend_s": max(maxima["Backend"], maxima["Total Backend"]), + } + return { + "parsed": parsed, + "headers": headers, + "templates": templates, + "tu_split": tu_split, + } + + +def build_report(run_dir, build_dir, top_n, build_status): + meta = read_meta(run_dir) + if not build_dir: + build_dir = meta.get("build_dir") + phases = read_phases(run_dir) + records = read_compile_log(run_dir) + ninja_edges = read_ninja_log(run_dir, build_dir) + + compiles = [r for r in records if r.get("kind") in ("compile", "pch")] + links = [r for r in records if r.get("kind") == "link"] + failed = [r for r in records if r.get("rc", 0) != 0] + + lines = [] + out = lines.append + out("=" * 78) + out(" Doris BE compile benchmark report run: {}".format( + meta.get("run_id", os.path.basename(run_dir.rstrip("/"))))) + out("=" * 78) + out(" build status : {}".format(build_status)) + for key in ("date_utc", "git_branch", "git_commit", "uname", "ncpu", "parallel", + "build_type", "generator", "toolchain", "cxx_version", "enable_pch", + "time_trace", "build_dir"): + if key in meta: + out(" {:<13}: {}".format(key, meta[key])) + + # ---- Phases ------------------------------------------------------------- + out("") + out(section("Phases")) + total_s = None + for phase in phases: + if phase["name"] == "total": + total_s = phase["dur_s"] + for phase in phases: + if phase["name"] == "total": + continue + pct = " ({:5.1f}%)".format(100.0 * phase["dur_s"] / total_s) if total_s else "" + out(" {:<24} {:>8}{}".format(phase["name"], fmt_dur(phase["dur_s"]), pct)) + if total_s is not None: + out(" {:<24} {:>8}".format("total (wall)", fmt_dur(total_s))) + + build_phase_s = None + for phase in phases: + if phase["name"] == "build": + build_phase_s = phase["dur_s"] + + # ---- Build summary ------------------------------------------------------ + out("") + out(section("Build summary")) + sum_wall = sum(r["wall_s"] for r in compiles) + sum_cpu = sum(r["user_s"] + r["sys_s"] for r in compiles) + out(" compile units (compile+pch) : {}".format(len(compiles))) + out(" sum of TU wall time : {}".format(fmt_dur(sum_wall))) + out(" sum of TU cpu time (user+sys) : {}".format(fmt_dur(sum_cpu))) + if build_phase_s: + out(" build phase wall : {}".format(fmt_dur(build_phase_s))) + edge_sum = sum_wall + sum(r["wall_s"] for r in links) + out(" effective parallelism : {:.1f}x (sum TU+link wall / build wall)" + .format(edge_sum / build_phase_s)) + if compiles: + slowest = max(compiles, key=lambda r: r["wall_s"]) + out(" slowest single TU : {} ({})".format( + fmt_dur(slowest["wall_s"]), rel_path(slowest.get("src"), meta))) + hungriest = max(compiles, key=lambda r: r.get("maxrss_mb", 0)) + out(" largest TU peak rss : {:.0f} MB ({})".format( + hungriest.get("maxrss_mb", 0), rel_path(hungriest.get("src"), meta))) + for link in sorted(links, key=lambda r: r["wall_s"], reverse=True)[:5]: + out(" link {:<24} : {} peak rss {:.0f} MB".format( + os.path.basename(link.get("out") or "?"), + fmt_dur(link["wall_s"]), link.get("maxrss_mb", 0))) + if failed: + out(" FAILED commands : {}".format(len(failed))) + for record in failed[:10]: + out(" rc={:<4} {}".format( + record.get("rc"), rel_path(record.get("src") or record.get("out"), meta))) + + # ---- Top slow TUs ------------------------------------------------------- + out("") + out(section("Top {} slowest translation units (wall)".format(top_n))) + out(" {:>4} {:>8} {:>8} {:>7} {:>9} {}".format( + "rank", "wall", "user", "sys", "maxrss", "file")) + ranked = sorted(compiles, key=lambda r: r["wall_s"], reverse=True) + for idx, record in enumerate(ranked[:top_n], 1): + out(" {:>4} {:>8} {:>8} {:>7} {:>7.0f}MB {}{}".format( + idx, fmt_dur(record["wall_s"]), fmt_dur(record["user_s"]), + fmt_dur(record["sys_s"]), record.get("maxrss_mb", 0), + rel_path(record.get("src"), meta), + " [pch]" if record.get("kind") == "pch" else "")) + + # ---- Directory rollup --------------------------------------------------- + for level, title in ((0, "top-level directory"), (1, "second-level directory")): + rollup = defaultdict(lambda: [0.0, 0]) + for record in compiles: + rel = rel_path(record.get("src") or record.get("out") or "?", meta) + key = group_keys(rel)[level] + entry = rollup[key] + entry[0] += record["wall_s"] + entry[1] += 1 + out("") + out(section("Wall time by {}".format(title))) + out(" {:>9} {:>6} {:>8} {}".format("wall-sum", "count", "avg", "directory")) + ordered = sorted(rollup.items(), key=lambda kv: kv[1][0], reverse=True) + for key, (wall, count) in ordered[:TOP_DIRS]: + out(" {:>9} {:>6} {:>8} {}".format( + fmt_dur(wall), count, fmt_dur(wall / count), key)) + + # ---- Ninja tail: what the build waits on at the end --------------------- + if ninja_edges: + out("") + out(section("Last finishers (critical-path tail, from .ninja_log)")) + out(" {:>10} {:>10} {:>8} {}".format("start", "end", "dur", "output")) + tail = sorted(ninja_edges.items(), key=lambda kv: kv[1][1], reverse=True) + for output, (start_ms, end_ms) in tail[:TOP_TAIL]: + out(" {:>10} {:>10} {:>8} {}".format( + fmt_dur(start_ms / 1000.0), fmt_dur(end_ms / 1000.0), + fmt_dur((end_ms - start_ms) / 1000.0), output)) + + # ---- Optional -ftime-trace analysis ------------------------------------- + trace = None + if meta.get("time_trace") == "ON" and build_dir: + trace = load_time_traces(compiles, meta, build_dir) + out("") + out(section("[-ftime-trace] parsed {} trace files".format(trace["parsed"]))) + if trace["parsed"]: + out("") + out(" Top headers by inclusive parse time (overlapping, ranks hotspots):") + out(" {:>9} {:>7} {:>8} {}".format("total", "count", "avg", "header")) + for path, (total, count) in sorted( + trace["headers"].items(), key=lambda kv: kv[1][0], + reverse=True)[:TOP_HEADERS]: + out(" {:>9} {:>7} {:>8} {}".format( + fmt_dur(total), count, fmt_dur(total / count), + shorten_header(path, meta))) + out("") + out(" Top template instantiations (inclusive):") + out(" {:>9} {:>7} {}".format("total", "count", "symbol")) + for symbol, (total, count) in sorted( + trace["templates"].items(), key=lambda kv: kv[1][0], + reverse=True)[:TOP_TEMPLATES]: + out(" {:>9} {:>7} {}".format(fmt_dur(total), count, symbol[:110])) + out("") + out(" Frontend (parse/instantiate) vs backend (codegen/opt) of slowest TUs:") + out(" {:>9} {:>9} {:>9} {}".format("total", "frontend", "backend", "file")) + for record in ranked[:15]: + split = trace["tu_split"].get(record.get("src") or "") + if not split: + continue + out(" {:>9} {:>9} {:>9} {}".format( + fmt_dur(split["total_s"]), fmt_dur(split["frontend_s"]), + fmt_dur(split["backend_s"]), rel_path(record.get("src"), meta))) + else: + out(" (no trace files found under {} - was the build dir wiped?)" + .format(build_dir)) + + out("") + out("=" * 78) + + summary = { + "meta": meta, + "build_status": build_status, + "phases": {p["name"]: round(p["dur_s"], 1) for p in phases}, + "totals": { + "compile_units": len(compiles), + "sum_tu_wall_s": round(sum_wall, 1), + "sum_tu_cpu_s": round(sum_cpu, 1), + "build_phase_s": round(build_phase_s, 1) if build_phase_s else None, + }, + "files": { + rel_path(r.get("src") or r.get("out"), meta): { + "wall_s": r["wall_s"], + "user_s": r["user_s"], + "maxrss_mb": r.get("maxrss_mb", 0), + "kind": r.get("kind"), + } + for r in compiles + links + }, + } + if trace and trace["parsed"]: + summary["headers_top"] = { + shorten_header(path, meta): round(total, 1) + for path, (total, _) in sorted( + trace["headers"].items(), key=lambda kv: kv[1][0], reverse=True)[:100] + } + summary["templates_top"] = { + symbol[:200]: round(total, 1) + for symbol, (total, _) in sorted( + trace["templates"].items(), key=lambda kv: kv[1][0], reverse=True)[:100] + } + return lines, summary + + +def cmd_report(args): Review 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. -- 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]
