diff --git a/CHANGELOG.md b/CHANGELOG.md index fe37dfc3..4aaced4f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,10 @@ ## Unreleased +### Configurable benchmark timeout per runner + +Set `timeout_minutes` in a runner's `bench_runner.toml` configuration to +override its benchmark job timeout. + ### Bugfixes #### Use PGO on weekly builds diff --git a/README.md b/README.md index 5d0a8174..9fffe79b 100644 --- a/README.md +++ b/README.md @@ -98,6 +98,12 @@ You may limit the number of cores used to build Python with the `use_cores` opti use_cores = 2 ``` +Set `timeout_minutes` to a positive integer to limit a runner's benchmark job runtime in minutes. If omitted, the workflow template's default applies (Linux: 1440; otherwise GitHub's default). + +``` +timeout_minutes = 210 +``` + ### Try a benchmarking run There are instructions for running a benchmarking action already in the `README.md` of your repo. Look there and give it a try! diff --git a/bench_runner/runners.py b/bench_runner/runners.py index 826a43fe..c98e6cf0 100644 --- a/bench_runner/runners.py +++ b/bench_runner/runners.py @@ -46,8 +46,16 @@ class Runner: # The number of cores to use to compile CPython. If not provided, `make -j` # will be used. use_cores: int | None = None + # Benchmark job timeout in minutes. None uses the workflow template's default. + timeout_minutes: int | None = None def __post_init__(self): + if self.timeout_minutes is not None and ( + type(self.timeout_minutes) is not int or self.timeout_minutes <= 0 + ): + raise ValueError( + f"Runner {self.nickname!r}: timeout_minutes must be a positive integer" + ) if self.github_runner_name is None: self.github_runner_name = self.name if self.plot is None: diff --git a/bench_runner/scripts/install.py b/bench_runner/scripts/install.py index 3e78e7f7..24f72e41 100644 --- a/bench_runner/scripts/install.py +++ b/bench_runner/scripts/install.py @@ -178,6 +178,12 @@ def generate__benchmark(src: Any) -> Any: runner_template["runs-on"].append(runner.github_runner_name) + if runner.timeout_minutes is not None: + # Keep the timeout immediately after runs-on in generated YAML. + runner_template.pop("timeout-minutes", None) + idx = list(runner_template).index("runs-on") + 1 + runner_template.insert(idx, "timeout-minutes", runner.timeout_minutes) + machine_clauses = [ f"inputs.machine == '{runner.name}'", "inputs.machine == '__really_all'", diff --git a/tests/data/bench_runner.toml b/tests/data/bench_runner.toml index aeb00e49..0d6552a8 100644 --- a/tests/data/bench_runner.toml +++ b/tests/data/bench_runner.toml @@ -7,6 +7,7 @@ os = "linux" arch = "x86_64" hostname = "pyperf" plot = { name = "Linux", color = "C0" } +timeout_minutes = 210 [longitudinal_plot] subplots = [ diff --git a/tests/test_config.py b/tests/test_config.py index 10cbfb9e..f3673491 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -2,6 +2,10 @@ import socket +import pytest + + +from bench_runner import config from bench_runner import runners @@ -16,3 +20,32 @@ def test_get_runner_for_hostname(monkeypatch): assert runner.os == "linux" assert runner.arch == "x86_64" assert runner.hostname == "pyperf" + assert runner.timeout_minutes == 210 + + +@pytest.mark.parametrize("timeout_minutes", [None, 1, 1440]) +def test_runner_timeout_minutes_valid(timeout_minutes): + runner = runners.Runner( + nickname="x", + os="linux", + arch="x86_64", + hostname="x", + timeout_minutes=timeout_minutes, + ) + assert runner.timeout_minutes == timeout_minutes + + +@pytest.mark.parametrize( + "timeout_minutes", ["0", "-1", "true", "false", "1.5", '"210"'] +) +def test_runner_timeout_minutes_invalid(tmp_path, timeout_minutes): + cfgpath = tmp_path / "bench_runner.toml" + contents = (DATA_PATH / "bench_runner.toml").read_text() + cfgpath.write_text( + contents.replace( + "timeout_minutes = 210", f"timeout_minutes = {timeout_minutes}" + ) + ) + + with pytest.raises(ValueError, match="timeout_minutes.*must be a positive integer"): + config.get_config(cfgpath) diff --git a/tests/test_install.py b/tests/test_install.py new file mode 100644 index 00000000..b2d024ce --- /dev/null +++ b/tests/test_install.py @@ -0,0 +1,43 @@ +from pathlib import Path + + +import pytest + + +from bench_runner import config +from bench_runner import runners +from bench_runner.scripts import install + + +DATA_PATH = Path(__file__).parent / "data" + + +@pytest.mark.parametrize( + "os, default_timeout", [("linux", 1440), ("darwin", None), ("windows", None)] +) +def test_benchmark_timeout_minutes(tmp_path, monkeypatch, os, default_timeout): + cfg = config.get_config(DATA_PATH / "bench_runner.toml") + configured = runners.Runner( + nickname="configured", + os=os, + arch="x86_64", + hostname="configured", + timeout_minutes=210, + ) + default = runners.Runner( + nickname="default", os=os, arch="x86_64", hostname="default" + ) + monkeypatch.setattr(cfg, "runners", {"configured": configured, "default": default}) + monkeypatch.setattr(config, "get_config", lambda: cfg) + + source = install.load_yaml(install.TEMPLATE_PATH / "_benchmark.src.yml") + output = tmp_path / "_benchmark.yml" + install.write_yaml(output, install.generate__benchmark(source), check=False) + jobs = install.load_yaml(output)["jobs"] + + assert jobs[f"benchmark-{configured.name}"]["timeout-minutes"] == 210 + default_job = jobs[f"benchmark-{default.name}"] + if default_timeout is None: + assert "timeout-minutes" not in default_job + else: + assert default_job["timeout-minutes"] == default_timeout