From 7d8009bc7b4a2813cee799c3999b834e0fcd7493 Mon Sep 17 00:00:00 2001 From: Derek Date: Tue, 6 Oct 2026 17:08:54 +1100 Subject: [PATCH] fix(rust): scope PGO and BOLT builds to the shipped binary Every cargo-pgo step now passes --bin for each binary packaging ships: the PGO instrument and optimise builds, and both BOLT builds. The no-split BOLT retry carries the same scope. Without a target filter, cargo builds every bin whose required features are on. Under --all-features that includes a feature-gated pgo-driver bin in the app package. It has no profile, so the profile-use compile logged about 2,200 "no profile data available" warnings per arch on dfe-transform-vector, and BOLT optimised a binary nothing ships. --bin filters targets only. Package selection and feature resolution are unchanged, so -p would not have helped: the driver is a bin in the same package. The build stage hands over every packaged binary, so a multi-binary app still gets all of them built in each step. The workload still profiles the first one. The plain-build fallback, used when cargo-pgo cannot be installed, is unchanged. The local smoke recipe in docs/runtime/pgo-bolt.md now passes --bin too. Closes #526 --- docs/runtime/pgo-bolt.md | 6 +- src/hyperi_ci/languages/rust/build.py | 5 +- src/hyperi_ci/languages/rust/pgo.py | 43 ++++++-- tests/unit/test_rust_pgo.py | 128 ++++++++++++++++++++++++ tests/unit/test_rust_subprocess_path.py | 17 +--- 5 files changed, 173 insertions(+), 26 deletions(-) diff --git a/docs/runtime/pgo-bolt.md b/docs/runtime/pgo-bolt.md index 424e1ae4..a53f0b2c 100644 --- a/docs/runtime/pgo-bolt.md +++ b/docs/runtime/pgo-bolt.md @@ -234,14 +234,14 @@ reuse the project's proto types and TLS config. ## Validating your workload locally -Before pushing a `.hyperi-ci.yaml` opt-in, test the pipeline locally: +Before pushing a `.hyperi-ci.yaml` opt-in, test the pipeline locally. CI passes `--bin` for each binary it ships on every `cargo pgo` step, so a feature-gated driver in the same package never compiles against the profile. Do the same here: ```bash cargo install cargo-pgo rustup component add llvm-tools-preview # 1. Instrument -cargo pgo build -- --features jemalloc +cargo pgo build -- --bin --features jemalloc # 2. Run your workload against the instrumented binary bash scripts/pgo-workload.sh ./target/x86_64-unknown-linux-gnu/release/ @@ -259,7 +259,7 @@ llvm-profdata show --topn=20 target/pgo-profiles/merged.profdata # Red flag: your startup / config-loading functions at the top. # 6. Build optimised -cargo pgo optimize build -- --features jemalloc +cargo pgo optimize build -- --bin --features jemalloc ``` If step 5 shows startup code at the top, your workload needs more diff --git a/src/hyperi_ci/languages/rust/build.py b/src/hyperi_ci/languages/rust/build.py index 00728fb2..0a25db53 100644 --- a/src/hyperi_ci/languages/rust/build.py +++ b/src/hyperi_ci/languages/rust/build.py @@ -1266,14 +1266,15 @@ def _build_for_target( "PGO requested but crate has no binaries -- falling back to plain build" ) else: - # Use the first binary for PGO (projects with multiple bins can - # extend this later; the common case is one binary per crate). + # The workload profiles the first binary; every cargo-pgo step still + # builds all the binaries packaging ships, and only those. from hyperi_ci.languages.rust.pgo import run_pgo_build return run_pgo_build( target=target, profile=profile, binary_name=binary_names[0], + shipped_binaries=binary_names, cwd=Path.cwd(), # RUST_FEATURES / RUST_ALL_FEATURES are where the PGO path # reads the declared features for its own cargo lines. diff --git a/src/hyperi_ci/languages/rust/pgo.py b/src/hyperi_ci/languages/rust/pgo.py index 872c2873..6adce182 100644 --- a/src/hyperi_ci/languages/rust/pgo.py +++ b/src/hyperi_ci/languages/rust/pgo.py @@ -71,6 +71,7 @@ def run_pgo_build( cwd: Path, extra_env: dict[str, str] | None = None, outcome: OptimizationOutcome | None = None, + shipped_binaries: list[str] | None = None, ) -> int: """Run the PGO (and optionally BOLT) pipeline for one target. @@ -88,6 +89,9 @@ def run_pgo_build( outcome: Filled in with the stages that actually completed, so the caller can report a skip that this function warns about but does not fail on. + shipped_binaries: Every binary packaging ships. Each `cargo pgo` + step builds these and nothing else. Defaults to + ``[binary_name]``. Returns: 0 on success, non-zero on failure. @@ -101,6 +105,10 @@ def run_pgo_build( env_features.get("RUST_FEATURES", ""), all_features=env_features.get("RUST_ALL_FEATURES") == "true", ) + cargo_args = [ + *_bin_scope_args(shipped_binaries or [binary_name]), + *feature_args, + ] if not _ensure_cargo_pgo_installed(): warn( @@ -147,7 +155,7 @@ def run_pgo_build( # 1. Instrumented build info(f"PGO: building instrumented binary for {target}") - instrument_args = ["build", "--", "--target", target, *feature_args] + instrument_args = ["build", "--", "--target", target, *cargo_args] rc = _run_cargo_pgo(instrument_args, cwd=cwd, extra_env=build_env) if rc != 0 and target.startswith("aarch64") and shutil.which("mold"): # Profile counters push a large binary's text past the +/-128 MB @@ -192,7 +200,7 @@ def run_pgo_build( # 3. Optimised build using profile data info(f"PGO: building optimised binary for {target}") rc = _run_cargo_pgo( - ["optimize", "--", "--target", target, *feature_args], + ["optimize", "--", "--target", target, *cargo_args], cwd=cwd, extra_env={**(build_env or {}), **_PROFILE_USE_ENV}, ) @@ -205,7 +213,7 @@ def run_pgo_build( # 4. BOLT (optional, Linux-only) if bolt: rc = _run_bolt( - target, feature_args, binary_name, profile, cwd, build_env, outcome + target, cargo_args, binary_name, profile, cwd, build_env, outcome ) if rc != 0: warn("BOLT step failed -- continuing with PGO-only optimised binary") @@ -219,6 +227,20 @@ def run_pgo_build( # --------------------------------------------------------------------------- +def _bin_scope_args(binaries: list[str]) -> list[str]: + """Render one ``--bin`` per shipped binary for a `cargo pgo` step. + + Without a target filter cargo builds every bin whose required features are + on, so `--all-features` compiles a feature-gated workload driver against a + profile that does not cover it (#526). ``--bin`` filters targets only, so + package selection and feature resolution are unchanged. + """ + args: list[str] = [] + for name in binaries: + args.extend(["--bin", name]) + return args + + def _run_plain_release_build( target: str, feature_args: list[str], @@ -965,7 +987,7 @@ def _bolt_optimize_args() -> list[str]: def _attempt_bolt( target: str, - feature_args: list[str], + cargo_args: list[str], binary_name: str, profile: OptimizationProfile, cwd: Path, @@ -976,6 +998,9 @@ def _attempt_bolt( ) -> int: """Run one BOLT pass: instrument -> workload -> optimise. + ``cargo_args`` follows ``--target`` on both builds: the ``--bin`` scope + and the feature flags, the same as the PGO steps. + `bolt build` emits `-bolt-instrumented`; the workload must run against THAT binary so BOLT collects its own branch profile. Skipping the workload (the old behaviour) left `bolt optimize` with nothing to @@ -1017,7 +1042,7 @@ def _attempt_bolt( "--", "--target", target, - *feature_args, + *cargo_args, ], cwd=cwd, extra_env=bolt_env, @@ -1060,7 +1085,7 @@ def _attempt_bolt( "--", "--target", target, - *feature_args, + *cargo_args, ], cwd=cwd, extra_env=bolt_env, @@ -1075,7 +1100,7 @@ def _attempt_bolt( def _run_bolt( target: str, - feature_args: list[str], + cargo_args: list[str], binary_name: str, profile: OptimizationProfile, cwd: Path, @@ -1099,7 +1124,7 @@ def _run_bolt( """ rc = _attempt_bolt( target, - feature_args, + cargo_args, binary_name, profile, cwd, @@ -1120,7 +1145,7 @@ def _run_bolt( ) return _attempt_bolt( target, - feature_args, + cargo_args, binary_name, profile, cwd, diff --git a/tests/unit/test_rust_pgo.py b/tests/unit/test_rust_pgo.py index 2e892b7d..852119cc 100644 --- a/tests/unit/test_rust_pgo.py +++ b/tests/unit/test_rust_pgo.py @@ -1815,6 +1815,7 @@ def _run_pgo_bolt_pipeline( bolt_enabled: bool = True, bolt_toolchain: bool = True, extra_env: dict[str, str] | None = None, + shipped_binaries: list[str] | None = None, ): """Run PGO (+BOLT) with a project env naming sccache; return cargo calls. @@ -1838,6 +1839,7 @@ def _run_pgo_bolt_pipeline( binary_name="my-bin", cwd=tmp_path, extra_env=extra_env or {"RUSTC_WRAPPER": "sccache"}, + shipped_binaries=shipped_binaries, ) assert rc == 0 return [(c.args[0], c.kwargs["extra_env"]) for c in cargo.call_args_list] @@ -1908,6 +1910,132 @@ def test_both_bolt_steps_build_on_the_pgo_layout( assert "--with-pgo" in args[: args.index("--")], args +def _cargo_build_args(args: list[str]) -> list[str]: + """What cargo-pgo forwards to `cargo build`: everything after `--`.""" + return args[args.index("--") + 1 :] + + +class TestBuildsAreScopedToTheShippedBinaries: + """Every cargo-pgo step builds the shipped binaries and nothing else (#526). + + An app's feature-gated workload driver compiled under `--all-features` in + the profile-use step, logging thousands of no-profile-data warnings for a + binary that never ships. + """ + + _ALL_FEATURES = {"RUSTC_WRAPPER": "sccache", "RUST_ALL_FEATURES": "true"} + + @pytest.mark.parametrize( + ("cargo_results", "steps"), + [ + ([0, 0, 0, 0], ["build", "optimize", "bolt build", "bolt optimize"]), + ( + [0, 0, 1, 0, 0], + ["build", "optimize", "bolt build", "bolt build", "bolt optimize"], + ), + ], + ids=["first", "no-split"], + ) + def test_every_step_names_the_binary_before_the_features( + self, tmp_path, cargo_results, steps + ) -> None: + calls = _run_pgo_bolt_pipeline( + tmp_path, cargo_results, extra_env=self._ALL_FEATURES + ) + assert [_step(args) for args, _ in calls] == steps + for args, _ in calls: + assert _cargo_build_args(args) == [ + "--target", + "x86_64-unknown-linux-gnu", + "--bin", + "my-bin", + "--all-features", + ], args + + def test_pgo_only_steps_are_scoped_too(self, tmp_path) -> None: + calls = _run_pgo_bolt_pipeline(tmp_path, [0, 0], bolt_enabled=False) + assert [_step(args) for args, _ in calls] == ["build", "optimize"] + for args, _ in calls: + assert _cargo_build_args(args)[2:4] == ["--bin", "my-bin"], args + + def test_every_shipped_binary_is_built_and_no_other(self, tmp_path) -> None: + calls = _run_pgo_bolt_pipeline( + tmp_path, + [0, 0, 0, 0], + extra_env=self._ALL_FEATURES, + shipped_binaries=["my-bin", "my-bin-admin"], + ) + assert len(calls) == 4 + for args, _ in calls: + forwarded = _cargo_build_args(args) + bins = [ + forwarded[i + 1] for i, arg in enumerate(forwarded) if arg == "--bin" + ] + assert bins == ["my-bin", "my-bin-admin"], args + assert "--workspace" not in forwarded + assert "-p" not in forwarded + + def test_an_empty_list_falls_back_to_the_profiled_binary(self, tmp_path) -> None: + calls = _run_pgo_bolt_pipeline( + tmp_path, [0, 0], bolt_enabled=False, shipped_binaries=[] + ) + for args, _ in calls: + assert _cargo_build_args(args).count("--bin") == 1, args + assert "my-bin" in _cargo_build_args(args), args + + def test_the_plain_fallback_is_unchanged(self, tmp_path) -> None: + with ( + patch.object(pgo, "_ensure_cargo_pgo_installed", return_value=False), + patch.object( + pgo, "run_cmd", return_value=MagicMock(returncode=0) + ) as run_cmd, + ): + rc = run_pgo_build( + target="x86_64-unknown-linux-gnu", + profile=_make_profile(allocator="system"), + binary_name="my-bin", + cwd=tmp_path, + shipped_binaries=["my-bin"], + ) + assert rc == 0 + assert run_cmd.call_args.args[0] == [ + "cargo", + "build", + "--release", + "--target", + "x86_64-unknown-linux-gnu", + ] + + def test_the_build_stage_hands_over_every_packaged_binary( + self, tmp_path, monkeypatch + ) -> None: + from hyperi_ci.languages.rust import build + + monkeypatch.chdir(tmp_path) + bin_dir = tmp_path / "target" / "x86_64-unknown-linux-gnu" / "release" + bin_dir.mkdir(parents=True) + (bin_dir / "my-bin").touch() + monkeypatch.setattr(build, "_ensure_target_installed", lambda _target: True) + monkeypatch.setattr( + build, "_detect_binary_names", lambda: ["my-bin", "my-bin-admin"] + ) + + with ( + patch.object(pgo, "_ensure_cargo_pgo_installed", return_value=True), + patch.object(pgo, "_run_cargo_pgo", return_value=0) as cargo, + patch.object(pgo, "_run_workload", return_value=0) as workload, + ): + rc = build._build_for_target( + "x86_64-unknown-linux-gnu", "", False, {}, profile=_make_profile() + ) + + assert rc == 0 + assert str(workload.call_args.args[2]).endswith("/my-bin") + for call in cargo.call_args_list: + forwarded = _cargo_build_args(call.args[0]) + assert forwarded[2:6] == ["--bin", "my-bin", "--bin", "my-bin-admin"] + + def _profile_settings(env: dict[str, str]) -> dict[str, str]: return { key: value for key, value in env.items() if key.startswith("CARGO_PROFILE_") diff --git a/tests/unit/test_rust_subprocess_path.py b/tests/unit/test_rust_subprocess_path.py index 395c1cf9..b00f210c 100644 --- a/tests/unit/test_rust_subprocess_path.py +++ b/tests/unit/test_rust_subprocess_path.py @@ -327,19 +327,12 @@ def test_every_pgo_and_bolt_compile(self, tmp_path, monkeypatch, launches) -> No shared = _expect(**self._PROJECT_ENV, CARGO_PROFILE_RELEASE_STRIP="none") profile_use = {**shared, "RUSTC_WRAPPER": ""} bolt = {**profile_use, _RUSTFLAGS: "-C link-arg=-fuse-ld=lld"} + scope = ["--target", _NATIVE, "--bin", "app"] expected = [ - (["cargo", "pgo", "build", "--", "--target", _NATIVE], shared), - (["cargo", "pgo", "optimize", "--", "--target", _NATIVE], profile_use), - ( - ["cargo", "pgo", "bolt", "build", "--with-pgo", "--"] - + ["--target", _NATIVE], - bolt, - ), - ( - ["cargo", "pgo", "bolt", "optimize", "--with-pgo", "--"] - + ["--target", _NATIVE], - bolt, - ), + (["cargo", "pgo", "build", "--", *scope], shared), + (["cargo", "pgo", "optimize", "--", *scope], profile_use), + (["cargo", "pgo", "bolt", "build", "--with-pgo", "--", *scope], bolt), + (["cargo", "pgo", "bolt", "optimize", "--with-pgo", "--", *scope], bolt), ] assert [(launch["args"], _watched_env(launch)) for launch in launches] == ( expected