Stop Holding Runners for a Pull Request's Universal Build and Tests

Holding two runners from a pull request's second arch until its
universal build and tests finished kept one idle through the slower
arch's build. Replaying the week of 10-03 against the real queue, that
idle capacity pushed the median pull request from under three hours to
over sixteen. Those jobs take minutes, so they now queue like any other
job, and a pull request holds a runner only for an arch that is still
building. Each arch then always needs exactly one runner.

Co-authored-by: raistlin7447 <kris.austin@gmail.com>

.github/workflows/build_check_cache.yml

@@ -143,7 +143,6 @@ jobs:
     env:
       WAIT_MINUTES: 240
       PRIORITY: ${{ contains(github.event.pull_request.labels.*.name, 'macos-priority') }}
-      ARCH: ${{ inputs.arch }}
     # Must exceed WAIT_MINUTES.
     timeout-minutes: 250
     # One line for every arch of every pull request, and only its first job polls.

scripts/ci_macos_admission.py

@@ -4,20 +4,20 @@
 Runs from a Linux job in build_check_cache.yml, once per macOS arch, in a single
 repo-wide line (a concurrency group with queue: max), so only the job at the
 front of the line polls. It lets its arch in when the runners every active Build
-all run holds or still needs, plus what letting this arch in adds to its own
-run, fit in MACOS_RUNNER_LIMIT:
+all run holds or still needs, plus one for this arch, fit in MACOS_RUNNER_LIMIT:
 
 - A push, nightly or manual run holds RESERVE runners until its macOS work is
   done. The jobs API lists only the jobs a run has reached, so what it still
   needs cannot be counted and is reserved instead.
-- A pull request run with both arches let in holds RESERVE runners until its
-  macOS work is done, which covers its universal build and tests that never
-  pass through the line and start only after both arch builds.
-- A pull request run with one arch let in holds one runner until that arch's
+- A pull request run holds one runner for each arch let in until that arch's
   app build is done, and none after. A run holding a runner while it waits for
   its other arch would deadlock: the other arch can sit in the line behind a
   pull request that waits for that runner.
 - A run holds at least the macOS jobs it has queued or running.
+
+A pull request's universal build and tests skip the line and are not held for
+in advance. They take minutes, and holding two runners for them through the
+slower arch's build left runners idle while other builds waited.
 - In the minutes around the nightly's cron time, RESERVE runners are held for it
   until its run appears.
 
@@ -28,8 +28,8 @@ waiting as holding a runner, so the next free runners go to it.
 Any API error repeated FAILURES_BEFORE_ADMIT times, and the WAIT_MINUTES limit,
 let the arch in, so a fault here never blocks pull requests.
 
-Environment: GH_TOKEN, REPO, WORKFLOW_REF, GITHUB_RUN_ID, ARCH, and optionally
-PRIORITY, MACOS_RUNNER_LIMIT, WAIT_MINUTES, POLL_SECONDS and GITHUB_API_URL.
+Environment: GH_TOKEN, REPO, WORKFLOW_REF, GITHUB_RUN_ID, and optionally PRIORITY,
+MACOS_RUNNER_LIMIT, WAIT_MINUTES, POLL_SECONDS and GITHUB_API_URL.
 """
 
 import datetime
@@ -41,8 +41,8 @@ import urllib.error
 import urllib.parse
 import urllib.request
 
-# A Build all run builds arm64 and x86_64 at the same time, then the universal
-# build and the macOS tests at the same time.
+# A push, nightly or manual run builds arm64 and x86_64 at the same time, then
+# the universal build and the macOS tests at the same time.
 RESERVE = 2
 
 # Must match the job names in build_all.yml and build_check_cache.yml.
@@ -97,10 +97,9 @@ def priority_waiting(jobs):
     return sum(1 for job in gates(jobs, (PRIORITY_GATE,)) if job["status"] != "completed")
 
 
-def run_demand(run, jobs, yield_to_priority=False, letting_in=None):
+def run_demand(run, jobs, yield_to_priority=False):
     """macOS runners a run holds or still needs. With yield_to_priority, each
-    priority arch still waiting counts as holding one. letting_in counts that arch
-    as let in."""
+    priority arch still waiting counts as holding one."""
     # A run with no jobs that is pending waits behind another run of its
     # concurrency group, which holds the runners for both.
     if not jobs and run["status"] in ("pending", "waiting"):
@@ -110,10 +109,7 @@ def run_demand(run, jobs, yield_to_priority=False, letting_in=None):
         return active
     if run["event"] != "pull_request":
         return max(active, RESERVE)
-    arches = admitted_arches(jobs) | ({letting_in} if letting_in else set())
-    if len(arches) >= RESERVE:
-        return max(active, RESERVE)
-    held = sum(1 for arch in arches if not arch_built(jobs, arch))
+    held = sum(1 for arch in admitted_arches(jobs) if not arch_built(jobs, arch))
     if yield_to_priority:
         held += priority_waiting(jobs)
     return max(active, held)
@@ -162,11 +158,10 @@ def latest_run(api, repo, workflow, **params):
     return runs[0] if runs else None
 
 
-def measure(api, repo, workflow, run_id, now, priority=False, arch=None):
-    """Total macOS runners held or needed, one line per run that holds any, and
-    how many more letting arch in adds to run_id. The priority line does not
-    count the priority runs waiting behind it."""
-    total, lines, need = 0, [], 1
+def measure(api, repo, workflow, run_id, now, priority=False):
+    """Total macOS runners held or needed, and one line per run that holds any.
+    The priority line does not count the priority runs waiting behind it."""
+    total, lines = 0, []
     # A run is listed as queued whenever one of its jobs waits for a runner, and
     # as pending whenever one waits in a concurrency group, so runs in any of these
     # can hold runners. A run can move between the lists between the calls.
@@ -177,8 +172,6 @@ def measure(api, repo, workflow, run_id, now, priority=False, arch=None):
                        filter="latest", per_page=100)["jobs"]
         own = run["id"] == run_id
         demand = run_demand(run, jobs, yield_to_priority=not priority and not own)
-        if own:
-            need = max(1, run_demand(run, jobs, letting_in=arch) - demand)
         if demand:
             total += demand
             waiting = ", priority, waiting" if priority_waiting(jobs) and not own else ""
@@ -191,7 +184,7 @@ def measure(api, repo, workflow, run_id, now, priority=False, arch=None):
         if not last or parse_time(last["created_at"]) < start:
             total += RESERVE
             lines.append(f"  {RESERVE}  the nightly, due at {NIGHTLY_UTC:%H:%M} UTC")
-    return total, lines, need
+    return total, lines
 
 
 def wait(measure_now, limit, wait_minutes, poll_seconds,
@@ -201,7 +194,7 @@ def wait(measure_now, limit, wait_minutes, poll_seconds,
     failures = 0
     while True:
         try:
-            total, lines, need = measure_now()
+            total, lines = measure_now()
         except (urllib.error.URLError, OSError, ValueError, KeyError, TypeError) as error:
             failures += 1
             log(f"::warning title=macOS admission::Could not read the queue ({error}).")
@@ -209,10 +202,10 @@ def wait(measure_now, limit, wait_minutes, poll_seconds,
                 return "the queue could not be read"
         else:
             failures = 0
-            log(f"{total} of {limit} macOS runners held or needed, and this arch needs {need}:")
+            log(f"{total} of {limit} macOS runners held or needed:")
             for line in lines:
                 log(line)
-            if total + need <= limit:
+            if total + 1 <= limit:
                 return "a runner is free"
         if clock() + poll_seconds >= deadline:
             return f"it waited {wait_minutes} minutes"
@@ -227,7 +220,7 @@ def main():
     limit = int(os.environ.get("MACOS_RUNNER_LIMIT") or 5)
     reason = wait(
         lambda: measure(api, repo, workflow, run_id, datetime.datetime.now(datetime.timezone.utc),
-                        priority=os.environ.get("PRIORITY") == "true", arch=os.environ["ARCH"]),
+                        priority=os.environ.get("PRIORITY") == "true"),
         limit,
         wait_minutes=int(os.environ.get("WAIT_MINUTES") or 240),
         poll_seconds=int(os.environ.get("POLL_SECONDS") or 180),

scripts/tests/test_ci_macos_admission.py

@@ -93,11 +93,17 @@ class RunDemandTest(unittest.TestCase):
         failed = job("build_macos_arch (arm64) / Build Deps / Build Deps", conclusion="failure")
         self.assertEqual(admission.run_demand(PR, [gate("arm64"), failed]), 0)
 
-    def test_pull_request_keeps_its_runners_for_its_later_jobs(self):
-        # Both arch builds are done and the universal build and tests are not
-        # listed yet: they will need both runners without passing the line.
-        jobs = [gate("arm64"), gate("x86_64"), build("arm64", "completed"), build("x86_64", "completed")]
-        self.assertEqual(admission.run_demand(PR, jobs), 2)
+    def test_both_arches_let_in_hold_only_what_still_builds(self):
+        # The universal build and tests are not held for in advance.
+        jobs = [gate("arm64"), gate("x86_64"), build("arm64", "completed"), build("x86_64")]
+        self.assertEqual(admission.run_demand(PR, jobs), 1)
+
+    def test_universal_build_and_tests_count_once_queued(self):
+        built = [gate("arm64"), gate("x86_64"), build("arm64", "completed"), build("x86_64", "completed")]
+        self.assertEqual(admission.run_demand(PR, built), 0)
+        queued = [job("Build macOS Universal / Build OrcaSlicer", "queued", None),
+                  job("macOS arm64 / Unit Tests", "in_progress", None)]
+        self.assertEqual(admission.run_demand(PR, built + queued), 2)
 
     def test_pull_request_releases_when_macos_is_done(self):
         jobs = [gate("arm64"), gate("x86_64"), UNIVERSAL_DONE, TESTS_DONE]
@@ -138,16 +144,16 @@ class PriorityTest(unittest.TestCase):
 
     def test_measure_from_each_line(self):
         api = FakeApi({("runs", "pending"): [dict(PR, status="pending")], jobs_path(2): self.WAITING})
-        total, lines, _ = admission.measure(api, "o/r", "build_all.yml", 99, NOON)
+        total, lines = admission.measure(api, "o/r", "build_all.yml", 99, NOON)
         self.assertEqual(total, admission.RESERVE)
         self.assertIn("priority, waiting", lines[0])
-        total, _, _ = admission.measure(api, "o/r", "build_all.yml", 99, NOON, priority=True)
+        total, _ = admission.measure(api, "o/r", "build_all.yml", 99, NOON, priority=True)
         self.assertEqual(total, 0)
 
     def test_own_waiting_run_is_not_counted(self):
         api = FakeApi({("runs", "pending"): [dict(PR, status="pending")], jobs_path(2): self.WAITING})
-        total, _, need = admission.measure(api, "o/r", "build_all.yml", 2, NOON, arch="arm64")
-        self.assertEqual((total, need), (0, 1))
+        total, _ = admission.measure(api, "o/r", "build_all.yml", 2, NOON)
+        self.assertEqual(total, 0)
 
 
 class FakeApi:
@@ -182,7 +188,7 @@ class MeasureTest(unittest.TestCase):
             jobs_path(2): [gate("arm64"), build("arm64")],
             jobs_path(3): [gate("arm64"), gate("x86_64"), build("arm64", "queued")],
         })
-        total, lines, _ = admission.measure(api, "o/r", "build_all.yml", 99, NOON)
+        total, lines = admission.measure(api, "o/r", "build_all.yml", 99, NOON)
         self.assertEqual(total, admission.RESERVE + 1 + admission.RESERVE)
         self.assertEqual(len(lines), 3)
 
@@ -190,25 +196,25 @@ class MeasureTest(unittest.TestCase):
         # One of its jobs waits in a concurrency group while its macOS builds run.
         pending_pr = dict(PR, status="pending")
         api = FakeApi({("runs", "pending"): [pending_pr], jobs_path(2): [gate("arm64"), build("arm64")]})
-        total, _, _ = admission.measure(api, "o/r", "build_all.yml", 99, NOON)
+        total, _ = admission.measure(api, "o/r", "build_all.yml", 99, NOON)
         self.assertEqual(total, 1)
 
     def test_a_push_waiting_behind_another_holds_nothing(self):
         api = FakeApi({("runs", "pending"): [dict(PUSH, status="pending")], jobs_path(1): []})
-        total, _, _ = admission.measure(api, "o/r", "build_all.yml", 99, NOON)
+        total, _ = admission.measure(api, "o/r", "build_all.yml", 99, NOON)
         self.assertEqual(total, 0)
 
     def test_reads_every_page_of_runs(self):
         runs = [dict(PR, id=i) for i in range(150)]
         responses = {("runs", "in_progress"): runs}
         responses.update({jobs_path(i): [gate("arm64")] for i in range(150)})
-        total, _, _ = admission.measure(FakeApi(responses), "o/r", "build_all.yml", 999, NOON)
+        total, _ = admission.measure(FakeApi(responses), "o/r", "build_all.yml", 999, NOON)
         self.assertEqual(total, 150)
 
     def test_reserves_for_the_nightly_until_its_run_appears(self):
         due = datetime.datetime(2026, 10, 10, 2, 10, tzinfo=UTC)
         yesterday = {"created_at": "2026-10-09T02:20:00Z"}
-        total, lines, _ = admission.measure(
+        total, lines = admission.measure(
             FakeApi({("runs", "schedule"): [yesterday]}), "OrcaSlicer/OrcaSlicer", "build_all.yml", 99, due)
         self.assertEqual(total, admission.RESERVE)
         self.assertIn("nightly", lines[0])
@@ -216,13 +222,13 @@ class MeasureTest(unittest.TestCase):
     def test_no_nightly_reserve_once_its_run_exists(self):
         due = datetime.datetime(2026, 10, 10, 2, 30, tzinfo=UTC)
         today = {"created_at": "2026-10-10T02:21:00Z"}
-        total, _, _ = admission.measure(
+        total, _ = admission.measure(
             FakeApi({("runs", "schedule"): [today]}), "OrcaSlicer/OrcaSlicer", "build_all.yml", 99, due)
         self.assertEqual(total, 0)
 
     def test_no_nightly_reserve_in_a_fork(self):
         due = datetime.datetime(2026, 10, 10, 2, 10, tzinfo=UTC)
-        total, _, _ = admission.measure(FakeApi({}), "fork/OrcaSlicer", "build_all.yml", 99, due)
+        total, _ = admission.measure(FakeApi({}), "fork/OrcaSlicer", "build_all.yml", 99, due)
         self.assertEqual(total, 0)
 
     def test_counts_a_run_in_both_lists_once(self):
@@ -231,25 +237,13 @@ class MeasureTest(unittest.TestCase):
             ("runs", "queued"): [dict(PUSH, status="queued")],
             jobs_path(1): [],
         })
-        total, _, _ = admission.measure(api, "o/r", "build_all.yml", 99, NOON)
+        total, _ = admission.measure(api, "o/r", "build_all.yml", 99, NOON)
         self.assertEqual(total, admission.RESERVE)
 
-    def own(self, jobs, arch):
-        api = FakeApi({("runs", "in_progress"): [PR], jobs_path(2): jobs})
-        total, _, need = admission.measure(api, "o/r", "build_all.yml", 2, NOON, arch=arch)
-        return total, need
-
-    def test_first_arch_needs_one(self):
-        self.assertEqual(self.own([gate("arm64", "in_progress", None)], "arm64"), (0, 1))
-
-    def test_second_arch_needs_one_while_the_first_builds(self):
+    def test_counts_its_own_run(self):
         jobs = [gate("arm64"), build("arm64"), gate("x86_64", "in_progress", None)]
-        self.assertEqual(self.own(jobs, "x86_64"), (1, 1))
-
-    def test_second_arch_needs_both_once_the_first_is_built(self):
-        # Letting it in makes the run hold both for its universal build and tests.
-        jobs = [gate("arm64"), build("arm64", "completed"), gate("x86_64", "in_progress", None)]
-        self.assertEqual(self.own(jobs, "x86_64"), (0, 2))
+        api = FakeApi({("runs", "in_progress"): [PR], jobs_path(2): jobs})
+        self.assertEqual(admission.measure(api, "o/r", "build_all.yml", 2, NOON)[0], 1)
 
     def test_a_run_whose_other_arch_waits_behind_this_one_does_not_block_it(self):
         # The line is A-arm64, B-arm64, A-x86_64. A's arm64 is built and its
@@ -260,8 +254,8 @@ class MeasureTest(unittest.TestCase):
             jobs_path(1): [gate("arm64"), build("arm64", "completed"), gate("x86_64", "pending", None)],
             jobs_path(2): [gate("arm64", "in_progress", None)],
         })
-        total, _, need = admission.measure(api, "o/r", "build_all.yml", 2, NOON, arch="arm64")
-        self.assertEqual((total, need), (0, 1))
+        total, _ = admission.measure(api, "o/r", "build_all.yml", 2, NOON)
+        self.assertEqual(total, 0)
 
     def test_no_nightly_lookup_outside_its_window(self):
         api = FakeApi({})
@@ -278,8 +272,7 @@ class WaitTest(unittest.TestCase):
             result = next(results)
             if isinstance(result, Exception):
                 raise result
-            total, need = result if isinstance(result, tuple) else (result, 1)
-            return total, [], need
+            return result, []
 
         def sleep(seconds):
             now[0] += seconds
@@ -294,8 +287,6 @@ class WaitTest(unittest.TestCase):
     def test_waits_while_full(self):
         self.assertEqual(self.run_wait([5, 6, 4]), ("a runner is free", 240))
 
-    def test_waits_for_what_this_arch_needs(self):
-        self.assertEqual(self.run_wait([(4, 2), (3, 2)]), ("a runner is free", 120))
 
     def test_lets_in_after_repeated_api_errors(self):
         error = urllib.error.URLError("rate limited")