From 0dd2a57ba4811307f2623c805efe5a16b2f11ea7 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Thu, 23 Apr 2026 14:13:01 -0500 Subject: [PATCH 01/11] [ci] Add import-time regression check for esphome.__main__ Adds a CI gate that runs `python -X importtime -c "import esphome.__main__"` via importtime-waterfall's best-of-N harness, compares the root cumulative time against a checked-in budget (script/import_time_budget.json, seeded at 75.2ms with a 15% margin), and fails the build when top-level imports regress. The CLI pays this cost on every invocation before the requested command even runs, so silently adding a top-level dep chain (the recent zeroconf move in #13135 being the motivating case) hurts every user. The check gives us a signal without waiting for user reports. - script/check_import_time.py: --check / --update / --har PATH modes. On regression, prints a ranked top-15 offenders table by self-time. - script/import_time_budget.json: baseline + margin_pct. - script/determine-jobs.py: should_run_import_time() gates the job on esphome/**/*.py, requirements.txt, requirements_dev.txt, pyproject.toml, or changes to the check itself. - .github/workflows/ci.yml: new import-time job, runs when gated and uploads a waterfall HAR artifact (14-day retention) for inspection. --- .github/workflows/ci.yml | 38 +++++ script/check_import_time.py | 260 +++++++++++++++++++++++++++++++++ script/determine-jobs.py | 37 +++++ script/import_time_budget.json | 5 + 4 files changed, 340 insertions(+) create mode 100755 script/check_import_time.py create mode 100644 script/import_time_budget.json diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 20c349ac00..03d0822e66 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -108,6 +108,42 @@ jobs: script/generate-esp32-boards.py --check script/generate-rp2040-boards.py --check + import-time: + name: Check import esphome.__main__ time + runs-on: ubuntu-24.04 + needs: + - common + - determine-jobs + if: needs.determine-jobs.outputs.import-time == 'true' + steps: + - name: Check out code from GitHub + uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 + - name: Restore Python + uses: ./.github/actions/restore-python + with: + python-version: ${{ env.DEFAULT_PYTHON }} + cache-key: ${{ needs.common.outputs.cache-key }} + - name: Install importtime-waterfall + run: | + . venv/bin/activate + pip install importtime-waterfall==1.0.0 + - name: Check import time against budget + run: | + . venv/bin/activate + script/check_import_time.py --check + - name: Generate waterfall HAR + if: always() + run: | + . venv/bin/activate + script/check_import_time.py --har importtime.har + - name: Upload waterfall HAR + if: always() + uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 + with: + name: import-time-waterfall + path: importtime.har + retention-days: 14 + pytest: name: Run pytest strategy: @@ -176,6 +212,7 @@ jobs: clang-tidy: ${{ steps.determine.outputs.clang-tidy }} clang-tidy-mode: ${{ steps.determine.outputs.clang-tidy-mode }} python-linters: ${{ steps.determine.outputs.python-linters }} + import-time: ${{ steps.determine.outputs.import-time }} changed-components: ${{ steps.determine.outputs.changed-components }} changed-components-with-tests: ${{ steps.determine.outputs.changed-components-with-tests }} directly-changed-components-with-tests: ${{ steps.determine.outputs.directly-changed-components-with-tests }} @@ -219,6 +256,7 @@ jobs: echo "clang-tidy=$(echo "$output" | jq -r '.clang_tidy')" >> $GITHUB_OUTPUT echo "clang-tidy-mode=$(echo "$output" | jq -r '.clang_tidy_mode')" >> $GITHUB_OUTPUT echo "python-linters=$(echo "$output" | jq -r '.python_linters')" >> $GITHUB_OUTPUT + echo "import-time=$(echo "$output" | jq -r '.import_time')" >> $GITHUB_OUTPUT echo "changed-components=$(echo "$output" | jq -c '.changed_components')" >> $GITHUB_OUTPUT echo "changed-components-with-tests=$(echo "$output" | jq -c '.changed_components_with_tests')" >> $GITHUB_OUTPUT echo "directly-changed-components-with-tests=$(echo "$output" | jq -c '.directly_changed_components_with_tests')" >> $GITHUB_OUTPUT diff --git a/script/check_import_time.py b/script/check_import_time.py new file mode 100755 index 0000000000..b170806fc8 --- /dev/null +++ b/script/check_import_time.py @@ -0,0 +1,260 @@ +#!/usr/bin/env python3 +"""Regression check for `import esphome.__main__` cost. + +Runs `python -X importtime -c "import esphome.__main__"` in fresh subprocesses +and compares the root cumulative import time against a checked-in budget +(`script/import_time_budget.json`). + +The CLI pays this cost on every invocation before the requested command even +runs, so a regression here hurts every user. Pair this with +`python -m importtime_waterfall --har` for human-readable waterfalls. +""" + +from __future__ import annotations + +import argparse +import json +from pathlib import Path +import subprocess +import sys +import time +from typing import Any, TextIO + +SCRIPT_DIR = Path(__file__).parent +BUDGET_PATH = SCRIPT_DIR / "import_time_budget.json" + +TARGET_MODULE = "esphome.__main__" +DEFAULT_RUNS = 3 +DEFAULT_MARGIN_PCT = 15 +OFFENDERS_TOP_N = 15 +IMPORT_TIME_PREFIX = "import time:" + + +def _run_importtime(module: str) -> tuple[float, str]: + """Run `python -X importtime -c 'import '` once. + + Returns (wall_seconds, stderr_text). + """ + before = time.monotonic() + result = subprocess.run( + [sys.executable, "-X", "importtime", "-c", f"import {module}"], + check=True, + stdout=subprocess.DEVNULL, + stderr=subprocess.PIPE, + text=True, + ) + return time.monotonic() - before, result.stderr + + +def measure(module: str, runs: int) -> str: + """Run `module` import `runs` times; return the fastest run's stderr.""" + best_wall, best_stderr = _run_importtime(module) + for _ in range(runs - 1): + wall, stderr = _run_importtime(module) + if wall < best_wall: + best_wall, best_stderr = wall, stderr + return best_stderr + + +def parse_trace(stderr: str) -> list[tuple[int, int, int, str]]: + """Parse `-X importtime` stderr into (depth, self_us, cumulative_us, name).""" + entries: list[tuple[int, int, int, str]] = [] + for line in stderr.splitlines(): + if not line.startswith(IMPORT_TIME_PREFIX): + continue + body = line[len(IMPORT_TIME_PREFIX) :] + parts = body.split("|") + if len(parts) != 3: + continue + try: + self_us = int(parts[0].strip()) + cumulative_us = int(parts[1].strip()) + except ValueError: + continue # header row + name_field = parts[2].rstrip("\n") + name = name_field.lstrip(" ") + depth = (len(name_field) - len(name)) // 2 + entries.append((depth, self_us, cumulative_us, name)) + return entries + + +def root_cumulative_us(entries: list[tuple[int, int, int, str]], module: str) -> int: + """Return the cumulative import time of `module` (the root probe).""" + for depth, _self_us, cumulative_us, name in reversed(entries): + if depth == 0 and name == module: + return cumulative_us + raise RuntimeError( + f"Did not find a root-level trace entry for {module!r}. Is it importable?" + ) + + +def top_offenders( + entries: list[tuple[int, int, int, str]], n: int +) -> list[tuple[str, int, int]]: + """Return up to `n` modules with the highest self-time, deduped by name. + + Returns list of (name, self_us, cumulative_us). A module imported from + multiple places is counted once, at its deepest-stacktrace occurrence + (same as what the user sees on the `--graph` output). + """ + seen: dict[str, tuple[int, int]] = {} + for _depth, self_us, cumulative_us, name in entries: + if name in seen: + continue + seen[name] = (self_us, cumulative_us) + ranked = sorted( + ((name, self_us, cum_us) for name, (self_us, cum_us) in seen.items()), + key=lambda row: row[1], + reverse=True, + ) + return ranked[:n] + + +def read_budget() -> dict[str, Any]: + if not BUDGET_PATH.exists(): + return {} + with BUDGET_PATH.open() as f: + return json.load(f) + + +def write_budget(cumulative_us: int, margin_pct: int) -> None: + payload = { + "target_module": TARGET_MODULE, + "margin_pct": margin_pct, + "cumulative_us": cumulative_us, + } + with BUDGET_PATH.open("w") as f: + json.dump(payload, f, indent=2) + f.write("\n") + + +def _format_us(us: int) -> str: + if us >= 1000: + return f"{us / 1000:.1f}ms" + return f"{us}us" + + +def _print_offenders_table( + offenders: list[tuple[str, int, int]], stream: TextIO +) -> None: + name_w = max(len(name) for name, _, _ in offenders) + print(f"\n{'module':<{name_w}} {'self':>10} {'cumulative':>12}", file=stream) + print(f"{'-' * name_w} {'-' * 10} {'-' * 12}", file=stream) + for name, self_us, cum_us in offenders: + print( + f"{name:<{name_w}} {_format_us(self_us):>10} {_format_us(cum_us):>12}", + file=stream, + ) + + +def cmd_check(args: argparse.Namespace) -> int: + budget = read_budget() + if not budget: + print( + f"ERROR: {BUDGET_PATH.name} missing. Run with --update first.", + file=sys.stderr, + ) + return 2 + + stderr = measure(TARGET_MODULE, args.runs) + entries = parse_trace(stderr) + measured = root_cumulative_us(entries, TARGET_MODULE) + + baseline = budget["cumulative_us"] + margin_pct = budget.get("margin_pct", DEFAULT_MARGIN_PCT) + ceiling = int(baseline * (1 + margin_pct / 100)) + + summary = ( + f"measured {TARGET_MODULE}: {_format_us(measured)} " + f"(budget {_format_us(baseline)} + {margin_pct}% = {_format_us(ceiling)})" + ) + + if measured <= ceiling: + print(summary) + return 0 + + print( + f"REGRESSION: `import {TARGET_MODULE}` took {_format_us(measured)}, " + f"exceeding the budget of {_format_us(baseline)} + {margin_pct}% " + f"({_format_us(ceiling)}).\n" + f"Top import-time offenders (by self time):", + file=sys.stderr, + ) + _print_offenders_table(top_offenders(entries, OFFENDERS_TOP_N), sys.stderr) + print( + "\nIf this regression is intentional, regenerate the budget with:\n" + " script/check_import_time.py --update\n" + "Otherwise, consider making the new import lazy " + "(import inside the function that uses it).", + file=sys.stderr, + ) + return 1 + + +def cmd_update(args: argparse.Namespace) -> int: + stderr = measure(TARGET_MODULE, args.runs) + entries = parse_trace(stderr) + measured = root_cumulative_us(entries, TARGET_MODULE) + write_budget(measured, args.margin_pct) + print( + f"Wrote {BUDGET_PATH.name}: " + f"{TARGET_MODULE}={_format_us(measured)} " + f"(margin {args.margin_pct}%)" + ) + return 0 + + +def cmd_har(args: argparse.Namespace) -> int: + out_path = Path(args.har) + result = subprocess.run( + [sys.executable, "-m", "importtime_waterfall", "--har", TARGET_MODULE], + check=True, + stdout=subprocess.PIPE, + text=True, + ) + out_path.write_text(result.stdout) + print(f"Wrote waterfall HAR to {out_path}") + return 0 + + +def main() -> int: + parser = argparse.ArgumentParser(description=__doc__) + parser.add_argument( + "--runs", + type=int, + default=DEFAULT_RUNS, + help=f"Number of measurement runs (default: {DEFAULT_RUNS}, best-of-N).", + ) + parser.add_argument( + "--margin-pct", + type=int, + default=DEFAULT_MARGIN_PCT, + help=(f"Margin over baseline for --update (default: {DEFAULT_MARGIN_PCT}%%)."), + ) + mode = parser.add_mutually_exclusive_group(required=True) + mode.add_argument( + "--check", action="store_true", help="Fail if measured time exceeds budget." + ) + mode.add_argument( + "--update", + action="store_true", + help="Rewrite the budget from a fresh measurement.", + ) + mode.add_argument( + "--har", + metavar="PATH", + help="Write a waterfall HAR file via `importtime_waterfall --har`.", + ) + args = parser.parse_args() + + if args.check: + return cmd_check(args) + if args.update: + return cmd_update(args) + if args.har: + return cmd_har(args) + return 2 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/script/determine-jobs.py b/script/determine-jobs.py index d94d472c9e..6c2ab95826 100755 --- a/script/determine-jobs.py +++ b/script/determine-jobs.py @@ -349,6 +349,41 @@ def should_run_python_linters(branch: str | None = None) -> bool: return _any_changed_file_endswith(branch, PYTHON_FILE_EXTENSIONS) +# Files outside esphome/**/*.py whose changes can affect `import esphome.__main__` +# cost. requirements.txt / pyproject.toml change the dependency graph pulled in +# by top-level imports; check_import_time.py itself changes the check's behavior. +IMPORT_TIME_TRIGGER_FILES = frozenset( + { + "requirements.txt", + "requirements_dev.txt", + "pyproject.toml", + "script/check_import_time.py", + "script/import_time_budget.json", + } +) + + +def should_run_import_time(branch: str | None = None) -> bool: + """Determine if the `import esphome.__main__` time regression check should run. + + Runs when any Python file under `esphome/` changes (those modules are + loaded transitively from `esphome.__main__`), when dependency + declarations change, or when the check script/budget itself changes. + + Args: + branch: Branch to compare against. If None, uses default. + + Returns: + True if the import-time check should run, False otherwise. + """ + for file in changed_files(branch): + if file.startswith("esphome/") and file.endswith(PYTHON_FILE_EXTENSIONS): + return True + if file in IMPORT_TIME_TRIGGER_FILES: + return True + return False + + def determine_cpp_unit_tests( branch: str | None = None, ) -> tuple[bool, list[str]]: @@ -773,6 +808,7 @@ def main() -> None: run_clang_tidy = should_run_clang_tidy(args.branch) run_clang_format = should_run_clang_format(args.branch) run_python_linters = should_run_python_linters(args.branch) + run_import_time = should_run_import_time(args.branch) changed_cpp_file_count = count_changed_cpp_files(args.branch) # Get changed components @@ -906,6 +942,7 @@ def main() -> None: "clang_tidy_mode": clang_tidy_mode, "clang_format": run_clang_format, "python_linters": run_python_linters, + "import_time": run_import_time, "changed_components": changed_components, "changed_components_with_tests": changed_components_with_tests, "directly_changed_components_with_tests": list(directly_changed_with_tests), diff --git a/script/import_time_budget.json b/script/import_time_budget.json new file mode 100644 index 0000000000..6b084e2620 --- /dev/null +++ b/script/import_time_budget.json @@ -0,0 +1,5 @@ +{ + "target_module": "esphome.__main__", + "margin_pct": 15, + "cumulative_us": 75199 +} From 900232be2134a691ed29ad4702546310843ab1e0 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Thu, 23 Apr 2026 14:20:31 -0500 Subject: [PATCH 02/11] [ci] Parse importtime-waterfall HAR JSON; raise budget for CI runner MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The previous parser walked `-X importtime` stderr by hand (string-splitting on `|`, indent-width math). Replace it with a load of the HAR JSON that importtime-waterfall already produces: each entry carries the module name, self-time, and cumulative — no tree reconstruction needed. Drops the hand-rolled retry loop too, since importtime-waterfall does best-of-6 internally. The committed budget (75.2ms) was seeded on a fast local machine; GHA runners measured 122.8ms, so the first CI run tripped the check. Reset the baseline to 123ms with a 25% margin (ceiling ~154ms) to absorb GHA variance. Tighten later once we have several data points. Adds unit tests for should_run_import_time across the trigger matrix and wires the new mock into the existing test_main_* suite. --- script/check_import_time.py | 124 ++++++++++------------------ script/import_time_budget.json | 4 +- tests/script/test_determine_jobs.py | 60 ++++++++++++++ 3 files changed, 106 insertions(+), 82 deletions(-) diff --git a/script/check_import_time.py b/script/check_import_time.py index b170806fc8..7b405475c3 100755 --- a/script/check_import_time.py +++ b/script/check_import_time.py @@ -1,13 +1,13 @@ #!/usr/bin/env python3 """Regression check for `import esphome.__main__` cost. -Runs `python -X importtime -c "import esphome.__main__"` in fresh subprocesses -and compares the root cumulative import time against a checked-in budget +Runs `python -m importtime_waterfall --har esphome.__main__` (which invokes +`-X importtime` in fresh subprocesses, best-of-N) and compares the root +cumulative import time against a checked-in budget (`script/import_time_budget.json`). The CLI pays this cost on every invocation before the requested command even -runs, so a regression here hurts every user. Pair this with -`python -m importtime_waterfall --har` for human-readable waterfalls. +runs, so a regression here hurts every user. """ from __future__ import annotations @@ -17,93 +17,72 @@ import json from pathlib import Path import subprocess import sys -import time from typing import Any, TextIO SCRIPT_DIR = Path(__file__).parent BUDGET_PATH = SCRIPT_DIR / "import_time_budget.json" TARGET_MODULE = "esphome.__main__" -DEFAULT_RUNS = 3 DEFAULT_MARGIN_PCT = 15 OFFENDERS_TOP_N = 15 -IMPORT_TIME_PREFIX = "import time:" -def _run_importtime(module: str) -> tuple[float, str]: - """Run `python -X importtime -c 'import '` once. +def run_waterfall(module: str) -> str: + """Run `importtime_waterfall --har ` and return the HAR JSON text. - Returns (wall_seconds, stderr_text). + `importtime_waterfall` itself runs the target in 6 fresh subprocesses + under `-X importtime` and emits the HAR of the fastest run. """ - before = time.monotonic() result = subprocess.run( - [sys.executable, "-X", "importtime", "-c", f"import {module}"], + [sys.executable, "-m", "importtime_waterfall", "--har", module], check=True, - stdout=subprocess.DEVNULL, - stderr=subprocess.PIPE, + stdout=subprocess.PIPE, text=True, ) - return time.monotonic() - before, result.stderr + return result.stdout -def measure(module: str, runs: int) -> str: - """Run `module` import `runs` times; return the fastest run's stderr.""" - best_wall, best_stderr = _run_importtime(module) - for _ in range(runs - 1): - wall, stderr = _run_importtime(module) - if wall < best_wall: - best_wall, best_stderr = wall, stderr - return best_stderr +def measure(module: str) -> dict[str, Any]: + """Return the parsed HAR for importing `module`.""" + return json.loads(run_waterfall(module)) -def parse_trace(stderr: str) -> list[tuple[int, int, int, str]]: - """Parse `-X importtime` stderr into (depth, self_us, cumulative_us, name).""" - entries: list[tuple[int, int, int, str]] = [] - for line in stderr.splitlines(): - if not line.startswith(IMPORT_TIME_PREFIX): - continue - body = line[len(IMPORT_TIME_PREFIX) :] - parts = body.split("|") - if len(parts) != 3: - continue - try: - self_us = int(parts[0].strip()) - cumulative_us = int(parts[1].strip()) - except ValueError: - continue # header row - name_field = parts[2].rstrip("\n") - name = name_field.lstrip(" ") - depth = (len(name_field) - len(name)) // 2 - entries.append((depth, self_us, cumulative_us, name)) - return entries +def _entries(har: dict[str, Any]) -> list[dict[str, Any]]: + return har["log"]["entries"] -def root_cumulative_us(entries: list[tuple[int, int, int, str]], module: str) -> int: - """Return the cumulative import time of `module` (the root probe).""" - for depth, _self_us, cumulative_us, name in reversed(entries): - if depth == 0 and name == module: - return cumulative_us +def root_cumulative_us(har: dict[str, Any], module: str) -> int: + """Return the cumulative import time (µs) of `module` from a HAR. + + The HAR `time` field is authored by importtime_waterfall using µs values + fed through `timedelta(milliseconds=...)`, so the number read back is the + original self/cumulative time in microseconds (labelled "ms" in HAR). + """ + for entry in _entries(har): + if entry["request"]["url"] == module: + return entry["time"] raise RuntimeError( - f"Did not find a root-level trace entry for {module!r}. Is it importable?" + f"No HAR entry for {module!r}. Is it importable with " + f"`python -c 'import {module}'`?" ) -def top_offenders( - entries: list[tuple[int, int, int, str]], n: int -) -> list[tuple[str, int, int]]: - """Return up to `n` modules with the highest self-time, deduped by name. +def top_offenders(har: dict[str, Any], n: int) -> list[tuple[str, int, int]]: + """Return up to `n` (name, self_us, cumulative_us), ranked by self_us desc. - Returns list of (name, self_us, cumulative_us). A module imported from - multiple places is counted once, at its deepest-stacktrace occurrence - (same as what the user sees on the `--graph` output). + A module imported from multiple places is counted once (first entry wins, + matching importtime's own de-duplication). """ seen: dict[str, tuple[int, int]] = {} - for _depth, self_us, cumulative_us, name in entries: + for entry in _entries(har): + name = entry["request"]["url"] if name in seen: continue + self_us = entry["timings"]["receive"] + cumulative_us = entry["time"] seen[name] = (self_us, cumulative_us) ranked = sorted( - ((name, self_us, cum_us) for name, (self_us, cum_us) in seen.items()), + ((name, s, c) for name, (s, c) in seen.items()), key=lambda row: row[1], reverse=True, ) @@ -156,9 +135,8 @@ def cmd_check(args: argparse.Namespace) -> int: ) return 2 - stderr = measure(TARGET_MODULE, args.runs) - entries = parse_trace(stderr) - measured = root_cumulative_us(entries, TARGET_MODULE) + har = measure(TARGET_MODULE) + measured = root_cumulative_us(har, TARGET_MODULE) baseline = budget["cumulative_us"] margin_pct = budget.get("margin_pct", DEFAULT_MARGIN_PCT) @@ -180,7 +158,7 @@ def cmd_check(args: argparse.Namespace) -> int: f"Top import-time offenders (by self time):", file=sys.stderr, ) - _print_offenders_table(top_offenders(entries, OFFENDERS_TOP_N), sys.stderr) + _print_offenders_table(top_offenders(har, OFFENDERS_TOP_N), sys.stderr) print( "\nIf this regression is intentional, regenerate the budget with:\n" " script/check_import_time.py --update\n" @@ -192,9 +170,8 @@ def cmd_check(args: argparse.Namespace) -> int: def cmd_update(args: argparse.Namespace) -> int: - stderr = measure(TARGET_MODULE, args.runs) - entries = parse_trace(stderr) - measured = root_cumulative_us(entries, TARGET_MODULE) + har = measure(TARGET_MODULE) + measured = root_cumulative_us(har, TARGET_MODULE) write_budget(measured, args.margin_pct) print( f"Wrote {BUDGET_PATH.name}: " @@ -205,26 +182,13 @@ def cmd_update(args: argparse.Namespace) -> int: def cmd_har(args: argparse.Namespace) -> int: - out_path = Path(args.har) - result = subprocess.run( - [sys.executable, "-m", "importtime_waterfall", "--har", TARGET_MODULE], - check=True, - stdout=subprocess.PIPE, - text=True, - ) - out_path.write_text(result.stdout) - print(f"Wrote waterfall HAR to {out_path}") + Path(args.har).write_text(run_waterfall(TARGET_MODULE)) + print(f"Wrote waterfall HAR to {args.har}") return 0 def main() -> int: parser = argparse.ArgumentParser(description=__doc__) - parser.add_argument( - "--runs", - type=int, - default=DEFAULT_RUNS, - help=f"Number of measurement runs (default: {DEFAULT_RUNS}, best-of-N).", - ) parser.add_argument( "--margin-pct", type=int, diff --git a/script/import_time_budget.json b/script/import_time_budget.json index 6b084e2620..97b4f7456c 100644 --- a/script/import_time_budget.json +++ b/script/import_time_budget.json @@ -1,5 +1,5 @@ { "target_module": "esphome.__main__", - "margin_pct": 15, - "cumulative_us": 75199 + "margin_pct": 25, + "cumulative_us": 123000 } diff --git a/tests/script/test_determine_jobs.py b/tests/script/test_determine_jobs.py index de239ee0b5..bef6625456 100644 --- a/tests/script/test_determine_jobs.py +++ b/tests/script/test_determine_jobs.py @@ -56,6 +56,13 @@ def mock_should_run_python_linters() -> Generator[Mock, None, None]: yield mock +@pytest.fixture +def mock_should_run_import_time() -> Generator[Mock, None, None]: + """Mock should_run_import_time from determine_jobs.""" + with patch.object(determine_jobs, "should_run_import_time") as mock: + yield mock + + @pytest.fixture def mock_determine_cpp_unit_tests() -> Generator[Mock, None, None]: """Mock determine_cpp_unit_tests from helpers.""" @@ -91,6 +98,7 @@ def test_main_all_tests_should_run( mock_should_run_clang_tidy: Mock, mock_should_run_clang_format: Mock, mock_should_run_python_linters: Mock, + mock_should_run_import_time: Mock, mock_changed_files: Mock, mock_determine_cpp_unit_tests: Mock, capsys: pytest.CaptureFixture[str], @@ -104,6 +112,7 @@ def test_main_all_tests_should_run( mock_should_run_clang_tidy.return_value = True mock_should_run_clang_format.return_value = True mock_should_run_python_linters.return_value = True + mock_should_run_import_time.return_value = True mock_determine_cpp_unit_tests.return_value = (False, ["wifi", "api", "sensor"]) # Mock changed_files to return non-component files (to avoid memory impact) @@ -158,6 +167,7 @@ def test_main_all_tests_should_run( assert output["clang_tidy_mode"] in ["nosplit", "split"] assert output["clang_format"] is True assert output["python_linters"] is True + assert output["import_time"] is True assert output["changed_components"] == ["wifi", "api", "sensor"] # changed_components_with_tests will only include components that actually have test files assert "changed_components_with_tests" in output @@ -189,6 +199,7 @@ def test_main_no_tests_should_run( mock_should_run_clang_tidy: Mock, mock_should_run_clang_format: Mock, mock_should_run_python_linters: Mock, + mock_should_run_import_time: Mock, mock_changed_files: Mock, mock_determine_cpp_unit_tests: Mock, capsys: pytest.CaptureFixture[str], @@ -202,6 +213,7 @@ def test_main_no_tests_should_run( mock_should_run_clang_tidy.return_value = False mock_should_run_clang_format.return_value = False mock_should_run_python_linters.return_value = False + mock_should_run_import_time.return_value = False mock_determine_cpp_unit_tests.return_value = (False, []) # Mock changed_files to return no component files @@ -241,6 +253,7 @@ def test_main_no_tests_should_run( assert output["clang_tidy_mode"] == "disabled" assert output["clang_format"] is False assert output["python_linters"] is False + assert output["import_time"] is False assert output["changed_components"] == [] assert output["changed_components_with_tests"] == [] assert output["component_test_count"] == 0 @@ -261,6 +274,7 @@ def test_main_with_branch_argument( mock_should_run_clang_tidy: Mock, mock_should_run_clang_format: Mock, mock_should_run_python_linters: Mock, + mock_should_run_import_time: Mock, mock_changed_files: Mock, mock_determine_cpp_unit_tests: Mock, capsys: pytest.CaptureFixture[str], @@ -274,6 +288,7 @@ def test_main_with_branch_argument( mock_should_run_clang_tidy.return_value = True mock_should_run_clang_format.return_value = False mock_should_run_python_linters.return_value = True + mock_should_run_import_time.return_value = True mock_determine_cpp_unit_tests.return_value = (False, ["mqtt"]) # Mock changed_files to return non-component files (to avoid memory impact) @@ -310,6 +325,7 @@ def test_main_with_branch_argument( mock_should_run_clang_tidy.assert_called_once_with("main") mock_should_run_clang_format.assert_called_once_with("main") mock_should_run_python_linters.assert_called_once_with("main") + mock_should_run_import_time.assert_called_once_with("main") # Check output captured = capsys.readouterr() @@ -322,6 +338,7 @@ def test_main_with_branch_argument( assert output["clang_tidy_mode"] in ["nosplit", "split"] assert output["clang_format"] is False assert output["python_linters"] is True + assert output["import_time"] is True assert output["changed_components"] == ["mqtt"] # changed_components_with_tests will only include components that actually have test files assert "changed_components_with_tests" in output @@ -597,6 +614,49 @@ def test_should_run_python_linters_with_branch() -> None: mock_changed.assert_called_once_with("release") +@pytest.mark.parametrize( + ("changed_files", "expected_result"), + [ + # esphome Python files trigger the check + (["esphome/__main__.py"], True), + (["esphome/components/wifi/__init__.py"], True), + (["esphome/core/config.py"], True), + (["esphome/types.pyi"], True), + # Dependency declarations and the check's own files trigger + (["requirements.txt"], True), + (["requirements_dev.txt"], True), + (["pyproject.toml"], True), + (["script/check_import_time.py"], True), + (["script/import_time_budget.json"], True), + # Mixed: any triggering file is enough + (["docs/README.md", "esphome/config.py"], True), + # Python files outside esphome/ don't trigger + (["script/some_other_script.py"], False), + (["tests/script/test_determine_jobs.py"], False), + # Non-Python changes don't trigger + (["esphome/core/component.cpp"], False), + (["tests/components/wifi/test.esp32-idf.yaml"], False), + (["README.md"], False), + ([], False), + ], +) +def test_should_run_import_time( + changed_files: list[str], expected_result: bool +) -> None: + """Test should_run_import_time function.""" + with patch.object(determine_jobs, "changed_files", return_value=changed_files): + result = determine_jobs.should_run_import_time() + assert result == expected_result + + +def test_should_run_import_time_with_branch() -> None: + """Test should_run_import_time with branch argument.""" + with patch.object(determine_jobs, "changed_files") as mock_changed: + mock_changed.return_value = [] + determine_jobs.should_run_import_time("release") + mock_changed.assert_called_once_with("release") + + @pytest.mark.parametrize( ("changed_files", "expected_result"), [ From 64e000de13aae7b91eed083de5c4ca49b2d83fc6 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Thu, 23 Apr 2026 14:22:33 -0500 Subject: [PATCH 03/11] [ci] Tighten import-time margin to 15% MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CI measured 126.2ms against 123ms baseline + 25% ceiling (153.8ms). That's 27.6ms of headroom — enough slack that a real regression could hide under it. Drop to 15% (ceiling 141.5ms, ~15ms headroom over observed). Still well above GHA runner variance, tight enough to catch the class of regression we care about (zeroconf-style top-level import additions). --- script/import_time_budget.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/script/import_time_budget.json b/script/import_time_budget.json index 97b4f7456c..1e656dc977 100644 --- a/script/import_time_budget.json +++ b/script/import_time_budget.json @@ -1,5 +1,5 @@ { "target_module": "esphome.__main__", - "margin_pct": 25, + "margin_pct": 15, "cumulative_us": 123000 } From 6b7e8341748811ceb77d63bd6d5b2ad100bdb2b3 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Thu, 23 Apr 2026 14:23:27 -0500 Subject: [PATCH 04/11] [ci] Always print top import-time offenders, not just on regression MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Seeing the top contributors every run — regression or not — gives reviewers and future-PR authors a running reference for which imports are currently most expensive. That's the data we need to prioritize lazy-import refactors, so there's no reason to hide it on pass. --- script/check_import_time.py | 41 +++++++++++++++++++++---------------- 1 file changed, 23 insertions(+), 18 deletions(-) diff --git a/script/check_import_time.py b/script/check_import_time.py index 7b405475c3..1c25a3cdd6 100755 --- a/script/check_import_time.py +++ b/script/check_import_time.py @@ -146,27 +146,32 @@ def cmd_check(args: argparse.Namespace) -> int: f"measured {TARGET_MODULE}: {_format_us(measured)} " f"(budget {_format_us(baseline)} + {margin_pct}% = {_format_us(ceiling)})" ) + passed = measured <= ceiling + stream = sys.stdout if passed else sys.stderr - if measured <= ceiling: + if passed: print(summary) - return 0 + else: + print( + f"REGRESSION: `import {TARGET_MODULE}` took {_format_us(measured)}, " + f"exceeding the budget of {_format_us(baseline)} + {margin_pct}% " + f"({_format_us(ceiling)}).", + file=stream, + ) - print( - f"REGRESSION: `import {TARGET_MODULE}` took {_format_us(measured)}, " - f"exceeding the budget of {_format_us(baseline)} + {margin_pct}% " - f"({_format_us(ceiling)}).\n" - f"Top import-time offenders (by self time):", - file=sys.stderr, - ) - _print_offenders_table(top_offenders(har, OFFENDERS_TOP_N), sys.stderr) - print( - "\nIf this regression is intentional, regenerate the budget with:\n" - " script/check_import_time.py --update\n" - "Otherwise, consider making the new import lazy " - "(import inside the function that uses it).", - file=sys.stderr, - ) - return 1 + print("\nTop import-time offenders (by self time):", file=stream) + _print_offenders_table(top_offenders(har, OFFENDERS_TOP_N), stream) + + if not passed: + print( + "\nIf this regression is intentional, regenerate the budget with:\n" + " script/check_import_time.py --update\n" + "Otherwise, consider making the new import lazy " + "(import inside the function that uses it).", + file=stream, + ) + return 1 + return 0 def cmd_update(args: argparse.Namespace) -> int: From b8c6bd69c9a7387e356e72dfd39e904f3d20c3b3 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Thu, 23 Apr 2026 14:42:18 -0500 Subject: [PATCH 05/11] [ci] Cache importtime-waterfall in the shared venv The import-time job previously did a `pip install` on every run (~1s and some noisy output). Moving the pin to `requirements_test.txt` bakes it into the venv that `common` already builds and caches, so the job restores it from the cache and skips the install step. --- .github/workflows/ci.yml | 4 ---- requirements_test.txt | 3 +++ 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 03d0822e66..49767de0b7 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -123,10 +123,6 @@ jobs: with: python-version: ${{ env.DEFAULT_PYTHON }} cache-key: ${{ needs.common.outputs.cache-key }} - - name: Install importtime-waterfall - run: | - . venv/bin/activate - pip install importtime-waterfall==1.0.0 - name: Check import time against budget run: | . venv/bin/activate diff --git a/requirements_test.txt b/requirements_test.txt index bb98375cb6..e0eb4396c5 100644 --- a/requirements_test.txt +++ b/requirements_test.txt @@ -12,3 +12,6 @@ pytest-asyncio==1.3.0 pytest-xdist==3.8.0 asyncmock==0.4.2 hypothesis==6.92.1 + +# Used by the import-time regression check (.github/workflows/ci.yml → import-time job) +importtime-waterfall==1.0.0 From 99c80b7dfa5858110cbb1ee91da4aecf7b29ce3e Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Thu, 23 Apr 2026 14:47:03 -0500 Subject: [PATCH 06/11] [ci] Address copilot review feedback on import-time check - determine-jobs: trigger on changes to requirements_test.txt too; that file is hashed into the venv cache key and installed during restore-python, so a change there can alter the import environment. - ci.yml: merge the --check and --har steps so we only run importtime-waterfall once per job (was measuring twice: ~7-8s of wasted CI time per run). Uses the new script/check_import_time.py --check --har combination; the HAR reflects the same measurement that produced the pass/fail decision. - script/check_import_time.py: refactor the CLI so --har is a standalone option rather than a mutually-exclusive mode. --check and --update each accept an optional --har PATH that writes the HAR from the same subprocess invocation. Plain --har is still supported for local use. - tests: add tests/script/test_check_import_time.py covering HAR parsing, root lookup, offender ranking/dedup, budget round-trip, and the three --check exit paths (pass, regression, missing budget) plus the new --check --har combined write. Add requirements_test.txt case to the should_run_import_time parametrized test. --- .github/workflows/ci.yml | 10 +- script/check_import_time.py | 40 ++++-- script/determine-jobs.py | 1 + tests/script/test_check_import_time.py | 191 +++++++++++++++++++++++++ tests/script/test_determine_jobs.py | 1 + 5 files changed, 222 insertions(+), 21 deletions(-) create mode 100644 tests/script/test_check_import_time.py diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 49767de0b7..d60bd6edc3 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -123,21 +123,17 @@ jobs: with: python-version: ${{ env.DEFAULT_PYTHON }} cache-key: ${{ needs.common.outputs.cache-key }} - - name: Check import time against budget + - name: Check import time against budget and write waterfall HAR run: | . venv/bin/activate - script/check_import_time.py --check - - name: Generate waterfall HAR - if: always() - run: | - . venv/bin/activate - script/check_import_time.py --har importtime.har + script/check_import_time.py --check --har importtime.har - name: Upload waterfall HAR if: always() uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 with: name: import-time-waterfall path: importtime.har + if-no-files-found: ignore retention-days: 14 pytest: diff --git a/script/check_import_time.py b/script/check_import_time.py index 1c25a3cdd6..0d5362c968 100755 --- a/script/check_import_time.py +++ b/script/check_import_time.py @@ -42,9 +42,16 @@ def run_waterfall(module: str) -> str: return result.stdout -def measure(module: str) -> dict[str, Any]: - """Return the parsed HAR for importing `module`.""" - return json.loads(run_waterfall(module)) +def measure(module: str, har_path: Path | None = None) -> dict[str, Any]: + """Return the parsed HAR for importing `module`. + + When `har_path` is given, also write the raw HAR JSON to that path so + callers can combine `--check` with `--har` without measuring twice. + """ + har_text = run_waterfall(module) + if har_path is not None: + har_path.write_text(har_text) + return json.loads(har_text) def _entries(har: dict[str, Any]) -> list[dict[str, Any]]: @@ -135,7 +142,7 @@ def cmd_check(args: argparse.Namespace) -> int: ) return 2 - har = measure(TARGET_MODULE) + har = measure(TARGET_MODULE, har_path=Path(args.har) if args.har else None) measured = root_cumulative_us(har, TARGET_MODULE) baseline = budget["cumulative_us"] @@ -175,7 +182,7 @@ def cmd_check(args: argparse.Namespace) -> int: def cmd_update(args: argparse.Namespace) -> int: - har = measure(TARGET_MODULE) + har = measure(TARGET_MODULE, har_path=Path(args.har) if args.har else None) measured = root_cumulative_us(har, TARGET_MODULE) write_budget(measured, args.margin_pct) print( @@ -186,7 +193,7 @@ def cmd_update(args: argparse.Namespace) -> int: return 0 -def cmd_har(args: argparse.Namespace) -> int: +def cmd_har_only(args: argparse.Namespace) -> int: Path(args.har).write_text(run_waterfall(TARGET_MODULE)) print(f"Wrote waterfall HAR to {args.har}") return 0 @@ -200,7 +207,16 @@ def main() -> int: default=DEFAULT_MARGIN_PCT, help=(f"Margin over baseline for --update (default: {DEFAULT_MARGIN_PCT}%%)."), ) - mode = parser.add_mutually_exclusive_group(required=True) + parser.add_argument( + "--har", + metavar="PATH", + help=( + "Write a waterfall HAR file at PATH. Can be combined with " + "--check or --update to reuse that run's measurement (avoids " + "measuring twice)." + ), + ) + mode = parser.add_mutually_exclusive_group() mode.add_argument( "--check", action="store_true", help="Fail if measured time exceeds budget." ) @@ -209,11 +225,6 @@ def main() -> int: action="store_true", help="Rewrite the budget from a fresh measurement.", ) - mode.add_argument( - "--har", - metavar="PATH", - help="Write a waterfall HAR file via `importtime_waterfall --har`.", - ) args = parser.parse_args() if args.check: @@ -221,8 +232,9 @@ def main() -> int: if args.update: return cmd_update(args) if args.har: - return cmd_har(args) - return 2 + return cmd_har_only(args) + parser.error("Specify at least one of --check, --update, or --har PATH.") + return 2 # unreachable; parser.error exits. Here to satisfy ruff RET503. if __name__ == "__main__": diff --git a/script/determine-jobs.py b/script/determine-jobs.py index 6c2ab95826..1842aba3a0 100755 --- a/script/determine-jobs.py +++ b/script/determine-jobs.py @@ -356,6 +356,7 @@ IMPORT_TIME_TRIGGER_FILES = frozenset( { "requirements.txt", "requirements_dev.txt", + "requirements_test.txt", "pyproject.toml", "script/check_import_time.py", "script/import_time_budget.json", diff --git a/tests/script/test_check_import_time.py b/tests/script/test_check_import_time.py new file mode 100644 index 0000000000..223c58002c --- /dev/null +++ b/tests/script/test_check_import_time.py @@ -0,0 +1,191 @@ +"""Unit tests for script/check_import_time.py.""" + +from __future__ import annotations + +import importlib.util +import json +import os +from pathlib import Path +import sys +from unittest.mock import patch + +import pytest + +# Load the script-under-test as `check_import_time` (it's a hyphenated path +# inside `script/` that mirrors the existing `determine_jobs` pattern). +script_dir = os.path.abspath( + os.path.join(os.path.dirname(__file__), "..", "..", "script") +) +sys.path.insert(0, script_dir) +spec = importlib.util.spec_from_file_location( + "check_import_time", os.path.join(script_dir, "check_import_time.py") +) +check_import_time = importlib.util.module_from_spec(spec) +spec.loader.exec_module(check_import_time) + + +def _entry(name: str, self_us: int, cumulative_us: int) -> dict: + """Build a minimal HAR entry matching `importtime_waterfall --har`.""" + return { + "request": {"url": name}, + "time": cumulative_us, + "timings": {"receive": self_us, "wait": cumulative_us - self_us}, + } + + +def _har(*entries: dict) -> dict: + return {"log": {"entries": list(entries)}} + + +def test_root_cumulative_us_returns_time_for_root_module() -> None: + har = _har( + _entry("dep_a", 500, 500), + _entry("dep_b", 300, 300), + _entry("esphome.__main__", 100, 1000), + ) + assert check_import_time.root_cumulative_us(har, "esphome.__main__") == 1000 + + +def test_root_cumulative_us_missing_module_raises() -> None: + har = _har(_entry("something.else", 100, 100)) + with pytest.raises(RuntimeError, match="No HAR entry for 'esphome.__main__'"): + check_import_time.root_cumulative_us(har, "esphome.__main__") + + +def test_top_offenders_ranks_by_self_time_descending() -> None: + har = _har( + _entry("small", 100, 100), + _entry("big", 5000, 5000), + _entry("medium", 2000, 2500), + ) + result = check_import_time.top_offenders(har, n=10) + assert [name for name, _, _ in result] == ["big", "medium", "small"] + assert result[0] == ("big", 5000, 5000) + + +def test_top_offenders_respects_n_limit() -> None: + har = _har(*[_entry(f"m{i}", i * 100, i * 100) for i in range(1, 20)]) + assert len(check_import_time.top_offenders(har, n=5)) == 5 + + +def test_top_offenders_dedupes_repeat_names_keeping_first() -> None: + har = _har( + _entry("pkg", 5000, 5000), + _entry("pkg", 100, 100), # reimport later in trace + _entry("other", 1000, 1000), + ) + result = check_import_time.top_offenders(har, n=10) + assert [name for name, _, _ in result] == ["pkg", "other"] + # First occurrence wins + assert ("pkg", 5000, 5000) in result + + +def test_format_us_switches_to_ms_at_threshold() -> None: + assert check_import_time._format_us(500) == "500us" + assert check_import_time._format_us(999) == "999us" + assert check_import_time._format_us(1000) == "1.0ms" + assert check_import_time._format_us(12345) == "12.3ms" + + +def test_read_write_budget_roundtrip(tmp_path: Path) -> None: + budget_path = tmp_path / "budget.json" + with patch.object(check_import_time, "BUDGET_PATH", budget_path): + assert check_import_time.read_budget() == {} + check_import_time.write_budget(cumulative_us=12345, margin_pct=20) + loaded = check_import_time.read_budget() + assert loaded["cumulative_us"] == 12345 + assert loaded["margin_pct"] == 20 + assert loaded["target_module"] == check_import_time.TARGET_MODULE + + +def test_cmd_check_passes_when_measured_within_ceiling( + tmp_path: Path, capsys: pytest.CaptureFixture[str] +) -> None: + budget_path = tmp_path / "budget.json" + budget_path.write_text( + json.dumps( + { + "target_module": check_import_time.TARGET_MODULE, + "margin_pct": 15, + "cumulative_us": 100000, # 100ms + } + ) + ) + # Measured 90ms: inside 100ms + 15% = 115ms ceiling + har = _har(_entry(check_import_time.TARGET_MODULE, 1000, 90000)) + args = type("A", (), {"har": None})() + with ( + patch.object(check_import_time, "BUDGET_PATH", budget_path), + patch.object(check_import_time, "measure", return_value=har), + ): + rc = check_import_time.cmd_check(args) + assert rc == 0 + out = capsys.readouterr().out + assert "measured esphome.__main__:" in out + assert "budget 100.0ms" in out + + +def test_cmd_check_fails_when_measured_exceeds_ceiling( + tmp_path: Path, capsys: pytest.CaptureFixture[str] +) -> None: + budget_path = tmp_path / "budget.json" + budget_path.write_text( + json.dumps( + { + "target_module": check_import_time.TARGET_MODULE, + "margin_pct": 15, + "cumulative_us": 100000, + } + ) + ) + # Measured 120ms: over 100ms + 15% = 115ms ceiling + har = _har( + _entry("offender_a", 10000, 10000), + _entry(check_import_time.TARGET_MODULE, 1000, 120000), + ) + args = type("A", (), {"har": None})() + with ( + patch.object(check_import_time, "BUDGET_PATH", budget_path), + patch.object(check_import_time, "measure", return_value=har), + ): + rc = check_import_time.cmd_check(args) + assert rc == 1 + err = capsys.readouterr().err + assert "REGRESSION" in err + assert "120.0ms" in err + assert "offender_a" in err # top offender table + + +def test_cmd_check_returns_2_when_budget_missing( + tmp_path: Path, capsys: pytest.CaptureFixture[str] +) -> None: + budget_path = tmp_path / "nonexistent.json" + args = type("A", (), {"har": None})() + with patch.object(check_import_time, "BUDGET_PATH", budget_path): + rc = check_import_time.cmd_check(args) + assert rc == 2 + assert "missing" in capsys.readouterr().err + + +def test_cmd_check_writes_har_when_path_given(tmp_path: Path) -> None: + budget_path = tmp_path / "budget.json" + budget_path.write_text( + json.dumps( + { + "target_module": check_import_time.TARGET_MODULE, + "margin_pct": 15, + "cumulative_us": 100000, + } + ) + ) + har_path = tmp_path / "out.har" + har_text = json.dumps(_har(_entry(check_import_time.TARGET_MODULE, 1000, 80000))) + args = type("A", (), {"har": str(har_path)})() + with ( + patch.object(check_import_time, "BUDGET_PATH", budget_path), + patch.object(check_import_time, "run_waterfall", return_value=har_text), + ): + rc = check_import_time.cmd_check(args) + assert rc == 0 + assert har_path.exists() + assert json.loads(har_path.read_text()) == json.loads(har_text) diff --git a/tests/script/test_determine_jobs.py b/tests/script/test_determine_jobs.py index bef6625456..d44fecb95d 100644 --- a/tests/script/test_determine_jobs.py +++ b/tests/script/test_determine_jobs.py @@ -625,6 +625,7 @@ def test_should_run_python_linters_with_branch() -> None: # Dependency declarations and the check's own files trigger (["requirements.txt"], True), (["requirements_dev.txt"], True), + (["requirements_test.txt"], True), (["pyproject.toml"], True), (["script/check_import_time.py"], True), (["script/import_time_budget.json"], True), From fa055b9e490fa0408fa1293a642d2ec8ddf094ac Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Thu, 23 Apr 2026 14:30:03 -0500 Subject: [PATCH 07/11] [core] Lazy-load zeroconf/writer/yaml_util in __main__ MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Every \`esphome\` CLI invocation pays the cost of whatever \`esphome/__main__.py\` imports at module scope before the requested command even runs. This moves three heavy imports into the functions that actually use them: - \`esphome.zeroconf\` (discover_mdns_devices): only needed for the name_add_mac_suffix OTA discovery path. - \`esphome.writer\`: only needed by compile/clean paths. - \`esphome.yaml_util\`: only needed by codegen, config dump, and rename. Local measurement drops from ~75ms to ~47ms (-37%) for a cold \`python -c 'import esphome.__main__'\`. The zeroconf chain alone accounts for most of the gain — the case that motivated the CI budget check. Also adds a module-level note explaining the intent so future PRs don't innocently promote these back to the top. Retargets four test patches from \`esphome.__main__.discover_mdns_devices\` to \`esphome.zeroconf.discover_mdns_devices\` now that the symbol is only bound inside the function that calls it. --- esphome/__main__.py | 25 +++++++++++++++++++++++-- tests/unit_tests/test_main.py | 8 ++++---- 2 files changed, 27 insertions(+), 6 deletions(-) diff --git a/esphome/__main__.py b/esphome/__main__.py index 8c80dab90a..7556773a01 100644 --- a/esphome/__main__.py +++ b/esphome/__main__.py @@ -21,7 +21,7 @@ import argcomplete # Note: Do not import modules from esphome.components here, as this would # cause them to be loaded before external components are processed, resulting # in the built-in version being used instead of the external component one. -from esphome import const, writer, yaml_util +from esphome import const import esphome.codegen as cg from esphome.config import iter_component_configs, read_config, strip_default_ids from esphome.const import ( @@ -72,7 +72,12 @@ from esphome.util import ( run_external_process, safe_print, ) -from esphome.zeroconf import discover_mdns_devices + +# Keep expensive imports (zeroconf, writer, yaml_util, etc.) out of this +# module's top level. Every `esphome` invocation — including fast paths +# like `esphome version` — pays the cost of what's imported here before +# any command runs. Import inside the function that needs it instead. +# `script/check_import_time.py` enforces a budget in CI. _LOGGER = logging.getLogger(__name__) @@ -241,6 +246,8 @@ def _discover_mac_suffix_devices() -> list[str] | None: """ if not (has_name_add_mac_suffix() and has_mdns() and has_non_ip_address()): return None + from esphome.zeroconf import discover_mdns_devices + _LOGGER.info("Discovering devices...") if not (discovered := discover_mdns_devices(CORE.name)): _LOGGER.warning( @@ -661,6 +668,8 @@ def run_miniterm(config: ConfigType, port: str, args) -> int: def wrap_to_code(name, comp): + from esphome import yaml_util + coro = coroutine(comp.to_code) @functools.wraps(comp.to_code) @@ -680,6 +689,8 @@ def wrap_to_code(name, comp): def write_cpp(config: ConfigType, native_idf: bool = False) -> int: + from esphome import writer + if not get_bool_env(ENV_NOGITIGNORE): writer.write_gitignore() @@ -702,6 +713,8 @@ def generate_cpp_contents(config: ConfigType) -> None: def write_cpp_file(native_idf: bool = False) -> int: + from esphome import writer + code_s = indent(CORE.cpp_main_section) writer.write_cpp(code_s) @@ -1180,6 +1193,8 @@ def command_wizard(args: ArgsProtocol) -> int | None: def command_config(args: ArgsProtocol, config: ConfigType) -> int | None: + from esphome import yaml_util + if not CORE.verbose: config = strip_default_ids(config) output = yaml_util.dump(config, args.show_secrets) @@ -1321,6 +1336,8 @@ def command_clean_mqtt(args: ArgsProtocol, config: ConfigType) -> int | None: def command_clean_all(args: ArgsProtocol) -> int | None: + from esphome import writer + try: writer.clean_all(args.configuration) except OSError as err: @@ -1336,6 +1353,8 @@ def command_version(args: ArgsProtocol) -> int | None: def command_clean(args: ArgsProtocol, config: ConfigType) -> int | None: + from esphome import writer + try: writer.clean_build() except OSError as err: @@ -1538,6 +1557,8 @@ def command_analyze_memory(args: ArgsProtocol, config: ConfigType) -> int: def command_rename(args: ArgsProtocol, config: ConfigType) -> int | None: + from esphome import yaml_util + new_name = args.name for c in new_name: if c not in ALLOWED_NAME_CHARS: diff --git a/tests/unit_tests/test_main.py b/tests/unit_tests/test_main.py index 8ec9e70cf8..fb8f206a1d 100644 --- a/tests/unit_tests/test_main.py +++ b/tests/unit_tests/test_main.py @@ -2605,7 +2605,7 @@ def test_choose_upload_log_host_discovers_mac_suffix_devices(tmp_path: Path) -> } with ( patch( - "esphome.__main__.discover_mdns_devices", return_value=discovered + "esphome.zeroconf.discover_mdns_devices", return_value=discovered ) as mock_discover, patch( "esphome.__main__.choose_prompt", return_value="mydevice-abc123.local" @@ -2653,7 +2653,7 @@ def test_choose_upload_log_host_mac_suffix_no_devices_found( ) with ( - patch("esphome.__main__.discover_mdns_devices", return_value={}), + patch("esphome.zeroconf.discover_mdns_devices", return_value={}), caplog.at_level(logging.WARNING, logger="esphome.__main__"), pytest.raises(EsphomeError), ): @@ -2686,7 +2686,7 @@ def test_choose_upload_log_host_default_ota_discovers_mac_suffix( "mydevice-def456.local": ["10.0.0.2"], } with patch( - "esphome.__main__.discover_mdns_devices", return_value=discovered + "esphome.zeroconf.discover_mdns_devices", return_value=discovered ) as mock_discover: result = choose_upload_log_host( default="OTA", @@ -2715,7 +2715,7 @@ def test_choose_upload_log_host_default_ota_no_suffix_discovery( name="mydevice", ) - with patch("esphome.__main__.discover_mdns_devices") as mock_discover: + with patch("esphome.zeroconf.discover_mdns_devices") as mock_discover: result = choose_upload_log_host( default="OTA", check_default=None, From fd63a67ad8485b42c7eb23239e7abc17340e1150 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Thu, 23 Apr 2026 14:34:17 -0500 Subject: [PATCH 08/11] [ci] Lower import-time budget after lazy-import refactor CI now measures 81.2ms after the zeroconf/writer/yaml_util lazy loads. Reset baseline to 82ms with the 15% margin (ceiling 94.3ms, ~13ms headroom for GHA variance). --- script/import_time_budget.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/script/import_time_budget.json b/script/import_time_budget.json index 1e656dc977..2f9cac038c 100644 --- a/script/import_time_budget.json +++ b/script/import_time_budget.json @@ -1,5 +1,5 @@ { "target_module": "esphome.__main__", "margin_pct": 15, - "cumulative_us": 123000 + "cumulative_us": 82000 } From 5d624c7f76224a3f8581b80037759006480f55ef Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Thu, 23 Apr 2026 14:38:54 -0500 Subject: [PATCH 09/11] [core] Lazy-load esphome.core.config from loader and config The top-level `import esphome.core.config` in `esphome/loader.py` (used only to register the "esphome" pseudo-component in `_COMPONENT_CACHE`) was pulling `esphome.automation` + `esphome.config_validation` eagerly into every `esphome` CLI invocation, even fast paths like `esphome version`. Move the cache registration into `_lookup_module`, triggered on first `get_component("esphome")`. Also defers the two `esphome.core.config` call sites inside `esphome/config.py` (CoreFinalValidateStep and validate_config), which were the other eager entry point. Local `import esphome.__main__` drops from ~49ms to ~46ms; more importantly, `esphome.core.config` / `esphome.automation` no longer appear in the top offenders table, and `esphome.config_validation` drops from 4.7ms self to 2.0ms self on CI. --- esphome/config.py | 9 ++++++++- esphome/loader.py | 14 ++++++++++++-- 2 files changed, 20 insertions(+), 3 deletions(-) diff --git a/esphome/config.py b/esphome/config.py index 641b6ec1b4..e2f1094611 100644 --- a/esphome/config.py +++ b/esphome/config.py @@ -25,7 +25,10 @@ from esphome.const import ( CONF_SUBSTITUTIONS, ) from esphome.core import CORE, DocumentRange, EsphomeError -import esphome.core.config as core_config + +# `esphome.core.config` is imported lazily at its two use sites below. +# It pulls in `esphome.automation` and `esphome.config_validation`, which +# dominate `esphome.__main__` startup cost when loaded eagerly here. import esphome.final_validate as fv from esphome.helpers import indent from esphome.loader import ComponentManifest, get_component, get_platform @@ -968,6 +971,8 @@ class CoreFinalValidateStep(ConfigValidationStep): if result.errors: return + import esphome.core.config as core_config + token = fv.full_config.set(result) with result.catch_error([CONF_ESPHOME]): if CONF_ESPHOME in result: @@ -1072,6 +1077,8 @@ def validate_config( return result # 2. Load partial core config + import esphome.core.config as core_config + result[CONF_ESPHOME] = config[CONF_ESPHOME] result.add_output_path([CONF_ESPHOME], CONF_ESPHOME) try: diff --git a/esphome/loader.py b/esphome/loader.py index 68664aaa26..839f75fa29 100644 --- a/esphome/loader.py +++ b/esphome/loader.py @@ -13,9 +13,13 @@ from typing import Any from esphome.const import SOURCE_FILE_EXTENSIONS from esphome.core import CORE -import esphome.core.config from esphome.types import ConfigType +# `esphome.core.config` is imported lazily in `_lookup_module` when the +# "esphome" pseudo-component is first resolved. It pulls in +# `esphome.automation` and `esphome.config_validation`, which together +# dominate `esphome.__main__` startup cost when loaded eagerly. + _LOGGER = logging.getLogger(__name__) @@ -202,6 +206,13 @@ def _lookup_module(domain: str, exception: bool) -> ComponentManifest | None: if domain in _COMPONENT_CACHE: return _COMPONENT_CACHE[domain] + if domain == "esphome": + import esphome.core.config + + manif = ComponentManifest(esphome.core.config) + _COMPONENT_CACHE[domain] = manif + return manif + try: module = importlib.import_module(f"esphome.components.{domain}") except ImportError as e: @@ -237,7 +248,6 @@ def get_platform(domain: str, platform: str) -> ComponentManifest | None: _COMPONENT_CACHE: dict[str, ComponentManifest] = {} CORE_COMPONENTS_PATH = (Path(__file__).parent / "components").resolve() -_COMPONENT_CACHE["esphome"] = ComponentManifest(esphome.core.config) def _replace_component_manifest(domain: str, manifest: ComponentManifest) -> None: From 87fa5a2d48ef10f35bfd441d35f2830f6ae63967 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Thu, 23 Apr 2026 14:47:50 -0500 Subject: [PATCH 10/11] [ci] Reset import-time baseline to CI-observed 91ms CI measured 90.6ms after the lazy-import refactor lands. Set baseline to 91ms with 15% margin (ceiling 104.65ms, ~14ms headroom for GHA variance over the observed measurement). --- script/import_time_budget.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/script/import_time_budget.json b/script/import_time_budget.json index 2f9cac038c..af3aa83511 100644 --- a/script/import_time_budget.json +++ b/script/import_time_budget.json @@ -1,5 +1,5 @@ { "target_module": "esphome.__main__", "margin_pct": 15, - "cumulative_us": 82000 + "cumulative_us": 91000 } From 52f47d0b91be2f8ea8fdfcfa9e0f6bbd933f6b68 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Thu, 23 Apr 2026 14:57:31 -0500 Subject: [PATCH 11/11] [core] Import yaml_util once per build in generate_cpp_contents Address copilot review: wrap_to_code is called once per component, so the lazy `from esphome import yaml_util` inside it was doing a (cached) sys.modules lookup per component. Move the import up to generate_cpp_contents, pass the module down, rename the helper to _wrap_to_code to mark it private. --- esphome/__main__.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/esphome/__main__.py b/esphome/__main__.py index 7556773a01..781bcd6288 100644 --- a/esphome/__main__.py +++ b/esphome/__main__.py @@ -667,9 +667,7 @@ def run_miniterm(config: ConfigType, port: str, args) -> int: return 0 -def wrap_to_code(name, comp): - from esphome import yaml_util - +def _wrap_to_code(name, comp, yaml_util): coro = coroutine(comp.to_code) @functools.wraps(comp.to_code) @@ -702,11 +700,13 @@ def write_cpp(config: ConfigType, native_idf: bool = False) -> int: def generate_cpp_contents(config: ConfigType) -> None: + from esphome import yaml_util + _LOGGER.info("Generating C++ source...") for name, component, conf in iter_component_configs(CORE.config): if component.to_code is not None: - coro = wrap_to_code(name, component) + coro = _wrap_to_code(name, component, yaml_util) CORE.add_job(coro, conf) CORE.flush_tasks()