Skip to content

Commit 7b2d19c

Browse files
committed
parallel-make-check: warn when a job runtime drifts >50% from "minutes"
The "minutes" field is only a scheduling estimate; when it goes stale it just packs the schedule a little worse, and there was no signal that a value needed updating. Emit a non-fatal warning when a config that explicitly sets "minutes" finishes more than 50% above or below it (a GitHub ::warning:: annotation in CI, a plain line locally) and flag the row in the step-summary table with the value to copy over. Configs that omit "minutes" keep riding the 1.0 default and are left alone. The warning never touches the exit status, so it cannot fail the job.
1 parent 68381e0 commit 7b2d19c

1 file changed

Lines changed: 34 additions & 2 deletions

File tree

.github/scripts/parallel-make-check.py

Lines changed: 34 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,9 @@
1919
# minutes expected duration, from the Minutes column of a previous
2020
# run's summary (default 1.0). Schedule weight only - configs
2121
# run longest-first and --shard balances shards by it; a stale
22-
# value just packs the schedule a little worse.
22+
# value just packs the schedule a little worse, but one that
23+
# drifts past +/-50% of the measured time draws a warning
24+
# (never a failure) so it is easy to spot and update.
2325
# user_settings header staged as <builddir>/user_settings.h before
2426
# configure (path relative to the source root); pair it with
2527
# --enable-usersettings in "configure"
@@ -88,6 +90,9 @@ class Config:
8890
check: bool = True
8991
prepare: list[list[str]] = field(default_factory=list)
9092
run: list[list[str]] = field(default_factory=list)
93+
# Whether "minutes" was given in the JSON (vs the 1.0 default); only an
94+
# explicit estimate is checked for >50% drift against the real time.
95+
minutes_provided: bool = False
9196

9297
SRCDIR = Path(__file__).resolve().parents[2]
9398
ON_GITHUB = os.environ.get("GITHUB_ACTIONS") == "true"
@@ -208,7 +213,8 @@ def load_configs(opts: argparse.Namespace,
208213
entry.get("ldflags", opts.ldflags),
209214
float(minutes), user_settings, check,
210215
list(entry.get("prepare", [])),
211-
list(entry.get("run", []))))
216+
list(entry.get("run", [])),
217+
minutes_provided="minutes" in entry))
212218
if not configs:
213219
error(f"{opts.json}: no configs")
214220
return configs
@@ -238,6 +244,23 @@ def dump(title: str, path: Path) -> None:
238244
sys.stdout.flush()
239245

240246

247+
def warn(msg: str) -> None:
248+
# GitHub surfaces ::warning:: as an annotation at the top of the run;
249+
# locally it is just a line. Informational only - never fails the run.
250+
print(f"::warning::{msg}" if ON_GITHUB else f"WARNING: {msg}")
251+
252+
253+
def stale_estimate(cfg: Config, minutes: float) -> bool:
254+
# "minutes" is only a scheduling estimate (configs run longest-first;
255+
# --shard balances by it), never a pass/fail bound. Flag a finished
256+
# config whose real time drifted past +/-50% of an explicitly given
257+
# estimate so stale values - which pack the schedule worse - are easy
258+
# to find and update. Configs that omit "minutes" ride the 1.0 default
259+
# placeholder and are left alone.
260+
return (cfg.minutes_provided
261+
and not 0.5 * cfg.minutes <= minutes <= 1.5 * cfg.minutes)
262+
263+
241264
def run_config(cfg: Config, opts: argparse.Namespace) -> tuple[str | None,
242265
float]:
243266
if opts.fail_fast and stop_event.is_set():
@@ -344,6 +367,10 @@ def record_failure(step: str) -> str:
344367
# One line per passing config; the full logs would bloat the CI
345368
# log (they stay in build-<name>/make-check.log).
346369
print(f"{cfg.name}: pass [{minutes:.1f} min]")
370+
if stale_estimate(cfg, minutes):
371+
warn(f"{cfg.name}: ran {minutes:.1f} min but \"minutes\" "
372+
f"says {cfg.minutes:g} (>50% off) - update it in the "
373+
f"config JSON")
347374
sys.stdout.flush()
348375
else:
349376
dump(f"{cfg.name}: FAIL ({failed}) [{minutes:.1f} min]", log)
@@ -364,6 +391,11 @@ def summarize(results: list[tuple[Config, str | None, float]],
364391
ok = f":x: FAIL ({failed})"
365392
else:
366393
ok = ":white_check_mark: pass"
394+
if stale_estimate(cfg, minutes):
395+
# Non-fatal nudge mirroring the per-config warning, kept in
396+
# the summary next to the Minutes value to copy over.
397+
ok += (f' :warning: "minutes" {cfg.minutes:g} is >50% off, '
398+
f"update to ~{minutes:.1f}")
367399
lines.append(f"| {cfg.name} | {ok} | {minutes:.1f} |")
368400
# Two views of how efficiently the pool used the machine: thread
369401
# occupancy is the time the workers spent running configs out of the

0 commit comments

Comments
 (0)