From a3b58ff00edac8c8e93d416d44636f07b362a238 Mon Sep 17 00:00:00 2001 From: suresh a Date: Fri, 6 Nov 2026 13:04:19 +0530 Subject: [PATCH 1/2] perf(build): remove quadratic lookups in ninja generation and scheduler NinjaBackend._write_ninja matched each executable `depends` entry by scanning every target, and rebuilt the cflags string for every source. The static-library lookup is now a Counter built once per manifest and the cflags line is built once per target. A count rather than a set keeps output byte-identical even when two static libraries share a name. run_graph ordered freed nodes with `order.index`, a linear scan per node. A position map gives the same order in O(1) per lookup. No behaviour change: generated build.ninja and compile_commands.json are byte-identical to master on synthetic projects (small and large) and on 3000 randomized configs in independent review. Measured (median, macOS, Python 3.14): ninja large (600 targets, 24k sources): 379.7 ms -> 195.0 ms scheduler fan-out n=4000, jobs=4: 98.5 ms -> 53.0 ms small projects: unchanged (~1 ms / ~3 ms) Adds tests for internal static-library linking, per-source cflags and parallel dispatch order, each confirmed to fail when the optimized line is broken. Co-Authored-By: Claude Opus 5.5 Signed-off-by: suresh a --- ebuild/build/ninja_backend.py | 23 +++++--- ebuild/core/scheduler.py | 5 +- tests/unit/test_ninja_backend.py | 96 ++++++++++++++++++++++++++++++++ tests/unit/test_scheduler.py | 19 +++++++ 4 files changed, 135 insertions(+), 8 deletions(-) diff --git a/ebuild/build/ninja_backend.py b/ebuild/build/ninja_backend.py index 31f8b69a..c55e416d 100644 --- a/ebuild/build/ninja_backend.py +++ b/ebuild/build/ninja_backend.py @@ -12,6 +12,7 @@ import shlex import subprocess import sys +from collections import Counter from dataclasses import dataclass, field from pathlib import Path from typing import Dict, List, Optional @@ -217,8 +218,16 @@ def _write_ninja(self) -> None: toolchain_ldflags = self._get_toolchain_ldflags() + # Built once so linking an executable does not rescan every target for + # each name in its `depends`. A count, not a set, keeps the output + # identical even when two static libraries share a name. + static_lib_counts = Counter( + t.name for t in self.config.targets if t.target_type == "static_library" + ) + for target in self.config.targets: cflags = self._resolve_target_cflags(target) + cflags_line = f" cflags = {' '.join(cflags)}" if cflags else None obj_files = [] for src in target.sources: @@ -227,8 +236,8 @@ def _write_ninja(self) -> None: lines.append( f"build {obj}: cc {escape_ninja_path(src)}" ) - if cflags: - lines.append(f" cflags = {' '.join(cflags)}") + if cflags_line: + lines.append(cflags_line) lines.append("") if target.target_type in ("executable", "test"): @@ -245,11 +254,11 @@ def _write_ninja(self) -> None: # Add internal library dependencies (static_library targets) for dep_name in target.depends: - for dep_target in self.config.targets: - if dep_target.name == dep_name and dep_target.target_type == "static_library": - dep_archives.append( - escape_ninja_path(self.build_dir / f"lib{dep_name}.a") - ) + count = static_lib_counts.get(dep_name, 0) + if count: + dep_archives.extend( + [escape_ninja_path(self.build_dir / f"lib{dep_name}.a")] * count + ) link_inputs = obj_files + dep_archives out = escape_ninja_path( diff --git a/ebuild/core/scheduler.py b/ebuild/core/scheduler.py index 6086eec9..b38b6664 100644 --- a/ebuild/core/scheduler.py +++ b/ebuild/core/scheduler.py @@ -78,6 +78,9 @@ def run_graph( dependents[dep].append(node) # Seeding and refilling from `order` keeps dispatch deterministic. + # `order.index` would be a linear scan per freed node; a position map makes + # the same ordering O(1) per lookup. + position: Dict[str, int] = {node: i for i, node in enumerate(order)} ready: List[str] = [n for n in order if not pending[n]] results: Dict[str, T] = {} failure: BaseException | None = None @@ -114,7 +117,7 @@ def run_graph( pending[dependent].discard(node) if not pending[dependent]: freed.append(dependent) - ready.extend(sorted(freed, key=order.index)) + ready.extend(sorted(freed, key=position.__getitem__)) # A failure stops new work; already-running nodes finish rather than # being abandoned mid-build. diff --git a/tests/unit/test_ninja_backend.py b/tests/unit/test_ninja_backend.py index 1ac5c983..05ef3607 100644 --- a/tests/unit/test_ninja_backend.py +++ b/tests/unit/test_ninja_backend.py @@ -289,6 +289,102 @@ def test_compile_commands_distinguishes_shared_source_entries(self, tmp_path): assert len(set(objects)) == 2, f"entries name the same object: {objects}" +class TestInternalLibraryLinking: + """An executable links the archive of each static library it depends on. + + The lookup from a `depends` name to a static_library target is built once + per manifest rather than rescanning every target per dependency; these + pin the output that lookup must keep producing. + """ + + @staticmethod + def _link_inputs(build_dir, app_name): + lines = (build_dir / "build.ninja").read_text(encoding="utf-8").splitlines() + out = escape_ninja_path(layout.executable_output_path(build_dir, app_name)) + [line] = [l for l in lines if l.startswith(f"build {out}: link ")] + return line.split(": link ", 1)[1].split(" ") + + def test_static_dependencies_are_linked_in_depends_order(self, tmp_path): + config = ProjectConfig( + name="linking", + version="1.0.0", + targets=[ + TargetConfig(name="app", target_type="executable", + sources=["main.c"], + depends=["beta", "shared", "alpha", "absent"]), + TargetConfig(name="alpha", target_type="static_library", + sources=["a.c"]), + TargetConfig(name="beta", target_type="static_library", + sources=["b.c"]), + TargetConfig(name="shared", target_type="shared_library", + sources=["s.c"]), + ], + source_dir=tmp_path, + ) + build_dir = tmp_path / "_build" + NinjaBackend(config, build_dir, _toolchain()).generate() + + inputs = self._link_inputs(build_dir, "app") + archives = [i for i in inputs if not i.endswith(".o")] + # Only static libraries become link inputs; a shared library or an + # unknown name contributes no archive. + assert archives == [ + escape_ninja_path(build_dir / "libbeta.a"), + escape_ninja_path(build_dir / "libalpha.a"), + ] + + def test_each_same_named_static_library_contributes_an_archive(self, tmp_path): + """Pins existing output: one archive input per matching static_library. + + Duplicate target names are not a working configuration, but the + manifest for one must not change because the lookup became a map. + """ + config = ProjectConfig( + name="dupes", + version="1.0.0", + targets=[ + TargetConfig(name="app", target_type="executable", + sources=["main.c"], depends=["a"]), + TargetConfig(name="a", target_type="static_library", + sources=["a1.c"]), + TargetConfig(name="a", target_type="static_library", + sources=["a2.c"]), + ], + source_dir=tmp_path, + ) + build_dir = tmp_path / "_build" + NinjaBackend(config, build_dir, _toolchain()).generate() + + archives = [i for i in self._link_inputs(build_dir, "app") if not i.endswith(".o")] + assert archives == [escape_ninja_path(build_dir / "liba.a")] * 2 + + def test_every_object_edge_carries_the_target_cflags(self, tmp_path): + config = ProjectConfig( + name="cflags", + version="1.0.0", + targets=[ + TargetConfig(name="app", target_type="executable", + sources=["one.c", "two.c", "three.c"], + defines=["MODE=2"]), + TargetConfig(name="bare", target_type="static_library", + sources=["x.c", "y.c"]), + ], + source_dir=tmp_path, + ) + build_dir = tmp_path / "_build" + NinjaBackend(config, build_dir, _toolchain()).generate() + + lines = (build_dir / "build.ninja").read_text(encoding="utf-8").splitlines() + cc_edges = [i for i, l in enumerate(lines) if l.startswith("build ") and ": cc " in l] + assert len(cc_edges) == 5 + for i in cc_edges: + if "/app/" in lines[i]: + assert lines[i + 1] == " cflags = -DMODE=2" + else: + # A target with no flags gets no cflags line at all. + assert lines[i + 1] == "" + + @pytest.mark.ebuild class TestNinjaPathEscaping: """Paths in build statements must be escaped for Ninja's lexer. diff --git a/tests/unit/test_scheduler.py b/tests/unit/test_scheduler.py index 4f72388c..ea580b0f 100644 --- a/tests/unit/test_scheduler.py +++ b/tests/unit/test_scheduler.py @@ -248,6 +248,25 @@ def test_sequential_dispatch_order_is_stable(self): runs.append(seen) assert all(r == runs[0] for r in runs) + def test_parallel_dispatch_follows_topological_order(self, monkeypatch): + """Nodes freed by one completion are submitted in topological order.""" + import ebuild.core.scheduler as scheduler + + submitted = [] + + class RecordingPool(scheduler.ThreadPoolExecutor): + def submit(self, fn, node): + submitted.append(node) + return super().submit(fn, node) + + monkeypatch.setattr(scheduler, "ThreadPoolExecutor", RecordingPool) + mids = [f"m{i}" for i in range(6)] + g = _graph([(m, "root") for m in mids] + [("sink", m) for m in mids]) + + run_graph(g, lambda n: n, jobs=2) + + assert submitted == g.topological_sort() + def test_results_complete_regardless_of_job_count(self): g = _graph([("d", "b"), ("d", "c"), ("b", "a"), ("c", "a")]) expected = {"a": "a", "b": "b", "c": "c", "d": "d"} From b319333b739e808a4747471c21a4bad01ee7add9 Mon Sep 17 00:00:00 2001 From: aswin Date: Fri, 6 Nov 2026 11:08:31 -0800 Subject: [PATCH 2/2] test(ninja): match the app target on Windows paths too test_every_object_edge_carries_the_target_cflags matched the app target with "/app/" in the build line. Ninja paths use backslashes on Windows, so the app edges never matched there and the test asserted a blank line where the cflags line is. Normalise separators before matching; the generator output is unchanged. Fixes the Windows-only failure of this test on PR #186 (CI job Test (Python 3.13, windows-2022), run 42181919513). --- tests/unit/test_ninja_backend.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/unit/test_ninja_backend.py b/tests/unit/test_ninja_backend.py index 05ef3607..e2f0779b 100644 --- a/tests/unit/test_ninja_backend.py +++ b/tests/unit/test_ninja_backend.py @@ -378,7 +378,8 @@ def test_every_object_edge_carries_the_target_cflags(self, tmp_path): cc_edges = [i for i, l in enumerate(lines) if l.startswith("build ") and ": cc " in l] assert len(cc_edges) == 5 for i in cc_edges: - if "/app/" in lines[i]: + # Ninja paths use backslashes on Windows; normalise before matching. + if "/app/" in lines[i].replace("\\", "/"): assert lines[i + 1] == " cflags = -DMODE=2" else: # A target with no flags gets no cflags line at all.