From d380651a9fd8867af89abd44b4b12a9bbdd39fe1 Mon Sep 17 00:00:00 2001 From: joaomarcos Date: Tue, 1 Sep 2026 23:23:02 -0300 Subject: [PATCH] fix(lazy-deps): byte-compile lazily installed backends at install time MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A pip/uv install writes .py sources and no __pycache__ — and reinstalling the same version still deletes the cache the previous copy had. Nothing in Hermes compiles them, so the whole compile is paid by whoever imports the package next. For a lazily installed backend that is the foreground of a user request, with nothing printed while it runs. Measured for anthropic==0.87.0 (541 modules) on cpython-3.12.13: the first import after an install costs 2.2-2.7s against 0.7-1.0s warm, and 10.5s under concurrent load. N per-profile daemons cold-starting together each pay it in full, because none of them has written the cache yet. Compile the freshly installed distributions in _venv_pip_install instead, on the success path of both the uv and pip tiers. The caller is already waiting on an installer there and can see why. Package directories are resolved from each distribution's own file list, so specs whose import name differs from their package name (python-telegram-bot -> telegram) are covered. Best-effort: a compile failure never invalidates an install that succeeded, and sys.dont_write_bytecode is honored. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01MhAnkrFktFdmZwf64fUYLE --- tests/tools/test_lazy_deps.py | 95 +++++++++++++++++++++++++++++++++++ tools/lazy_deps.py | 81 ++++++++++++++++++++++++++++- 2 files changed, 174 insertions(+), 2 deletions(-) diff --git a/tests/tools/test_lazy_deps.py b/tests/tools/test_lazy_deps.py index 74838a69a3..8c4b0c40ae 100644 --- a/tests/tools/test_lazy_deps.py +++ b/tests/tools/test_lazy_deps.py @@ -483,3 +483,98 @@ class TestInstallSpecs: result = ld.install_specs(["honcho-ai==2.2.0"]) assert result.ok is False assert "disk on fire" in result.stderr + + +# --------------------------------------------------------------------------- +# Post-install bytecode warm (#100461) +# --------------------------------------------------------------------------- + + +class TestWarmInstalledBytecode: + """A pip/uv install leaves ``.py`` sources with no ``__pycache__``. + + Whoever imports next pays the whole compile, and for a lazily installed + backend that is the foreground of a user request. These tests pin that + the installer pays it instead. + """ + + @staticmethod + def _package(tmp_path): + pkg = tmp_path / "zzzfakepkg" + pkg.mkdir() + (pkg / "__init__.py").write_text("VALUE = 1\n", encoding="utf-8") + (pkg / "mod.py").write_text("def f():\n return 2\n", encoding="utf-8") + return pkg + + def test_compiles_the_installed_package(self, tmp_path, monkeypatch): + pkg = self._package(tmp_path) + monkeypatch.setattr(ld, "_installed_dist_roots", lambda spec, target: {pkg}) + + assert not list(pkg.rglob("*.pyc")) + ld._warm_installed_bytecode(("zzzfake==1.0",), None) + assert len(list(pkg.rglob("*.pyc"))) == 2 + + def test_honors_dont_write_bytecode(self, tmp_path, monkeypatch): + pkg = self._package(tmp_path) + monkeypatch.setattr(ld, "_installed_dist_roots", lambda spec, target: {pkg}) + monkeypatch.setattr(ld.sys, "dont_write_bytecode", True) + + ld._warm_installed_bytecode(("zzzfake==1.0",), None) + assert not list(pkg.rglob("*.pyc")) + + def test_compile_failure_never_propagates(self, tmp_path, monkeypatch): + # An unwritable tree (read-only mount, --target on a sealed image) + # must not turn a successful install into a failed one. + def boom(spec, target): + raise OSError("read-only file system") + monkeypatch.setattr(ld, "_installed_dist_roots", boom) + + ld._warm_installed_bytecode(("zzzfake==1.0",), None) # no exception + + def test_dist_roots_resolve_from_metadata_not_the_spec_name(self): + # The import name is read off the distribution's own file list, so + # specs whose package name differs from their module name still warm. + roots = ld._installed_dist_roots("pytest>=8", None) + assert roots, "pytest is a test dependency and must resolve" + assert all(r.is_dir() for r in roots) + assert any(list(r.glob("*.py")) for r in roots) + + def test_unknown_distribution_resolves_to_nothing(self): + assert ld._installed_dist_roots("zzz-not-installed==9.9", None) == set() + + +class TestInstallWarmsBytecode: + """The warm runs on install success, and only on success.""" + + @staticmethod + def _install(monkeypatch, returncode): + calls = [] + monkeypatch.setattr(ld, "_lazy_install_target", lambda: None) + monkeypatch.setattr(ld.shutil, "which", lambda name: "uv" if name == "uv" else None) + monkeypatch.setattr( + "hermes_cli.managed_uv.resolve_uv", lambda *a, **kw: "uv", raising=False + ) + + class _Completed: + def __init__(self): + self.returncode = returncode + self.stdout = "out" + self.stderr = "err" + + monkeypatch.setattr(ld.subprocess, "run", lambda *a, **kw: _Completed()) + monkeypatch.setattr( + ld, "_warm_installed_bytecode", + lambda specs, target: calls.append((specs, target)), + ) + result = ld._venv_pip_install(("zzzfake==1.0",)) + return result, calls + + def test_success_warms_once_with_the_installed_specs(self, monkeypatch): + result, calls = self._install(monkeypatch, 0) + assert result.success is True + assert calls == [(("zzzfake==1.0",), None)] + + def test_failed_install_does_not_warm(self, monkeypatch): + result, calls = self._install(monkeypatch, 1) + assert result.success is False + assert calls == [] diff --git a/tools/lazy_deps.py b/tools/lazy_deps.py index 64d4f2ec31..50e35dde9a 100644 --- a/tools/lazy_deps.py +++ b/tools/lazy_deps.py @@ -699,6 +699,80 @@ def _core_constraints_file() -> Optional[Path]: return None +def _installed_dist_roots(spec: str, target: Optional[Path]) -> set[Path]: + """Return the package directories a freshly installed *spec* owns. + + Resolved from the distribution's own file list rather than guessing the + import name from the spec — ``python-telegram-bot`` ships ``telegram``, + ``firecrawl-anydoc`` ships ``anydoc``, and several specs ship more than + one top-level package. + """ + name = _pkg_name_from_spec(spec) + try: + import importlib.metadata as _md + + if target is not None: + dists = list(_md.distributions(name=name, path=[str(target)])) + dist = dists[0] if dists else None + else: + dist = _md.distribution(name) + except Exception: + return set() + if dist is None: + return set() + + roots: set[Path] = set() + try: + for entry in dist.files or (): + parts = entry.parts + if not parts or parts[0].startswith(".") or parts[0] == "__pycache__": + continue + root = Path(dist.locate_file(parts[0])) + if root.is_dir(): + roots.add(root) + except Exception: + return set() + return roots + + +def _warm_installed_bytecode(specs: tuple[str, ...], target: Optional[Path]) -> None: + """Byte-compile what we just installed, so no user request has to. + + A pip/uv install writes ``.py`` sources and no ``__pycache__`` — and an + install of the *same* version still deletes the cache the old copy had. + Whoever imports the package next pays the whole compile: for + ``anthropic==0.87.0`` (541 modules) on cpython-3.12.13 that measured + 2.2-2.7s cold against 0.7-1.0s warm, and 10.5s cold under concurrent + load. That bill lands wherever the first import happens, and + for a lazily-installed backend that is the foreground of a user request + (#100461) — with nothing printed while it runs, so it reads as a hang. + Worse, N per-profile daemons cold-starting together each pay it in full + before any of them has written the cache. + + Paying it here instead is strictly better: the caller is already waiting + on an installer and can see why. Best-effort — a compile failure never + invalidates an install that succeeded. + """ + if sys.dont_write_bytecode: + return + try: + import compileall + except Exception: # pragma: no cover — stdlib, but never break an install + return + + for spec in specs: + try: + roots = _installed_dist_roots(spec, target) + except Exception as exc: + logger.debug("Bytecode warm skipped for %s: %s", spec, exc) + continue + for root in roots: + try: + compileall.compile_dir(str(root), quiet=2, force=False, workers=1) + except Exception as exc: + logger.debug("Bytecode warm skipped for %s: %s", root, exc) + + def _venv_pip_install(specs: tuple[str, ...], *, timeout: int = 300) -> _InstallResult: """Install ``specs`` using the uv → pip → ensurepip ladder. @@ -765,6 +839,7 @@ def _venv_pip_install(specs: tuple[str, ...], *, timeout: int = 300) -> _Install if r.returncode == 0: if target is not None: _activate_target_on_syspath(target) + _warm_installed_bytecode(specs, target) return _InstallResult(True, r.stdout or "", r.stderr or "") logger.debug("uv pip install failed: %s", r.stderr) # A resolver failure is authoritative. Falling through to pip @@ -810,8 +885,10 @@ def _venv_pip_install(specs: tuple[str, ...], *, timeout: int = 300) -> _Install stdin=subprocess.DEVNULL, creationflags=windows_hide_flags(), ) - if r.returncode == 0 and target is not None: - _activate_target_on_syspath(target) + if r.returncode == 0: + if target is not None: + _activate_target_on_syspath(target) + _warm_installed_bytecode(specs, target) return _InstallResult(r.returncode == 0, r.stdout or "", r.stderr or "") except subprocess.TimeoutExpired as e: return _InstallResult(False, "", f"pip install timed out: {e}")