diff --git a/.github/workflows/benchmark_backfill.yml b/.github/workflows/benchmark_backfill.yml index 44348bffa1..9b09dd4c5c 100644 --- a/.github/workflows/benchmark_backfill.yml +++ b/.github/workflows/benchmark_backfill.yml @@ -10,6 +10,11 @@ on: description: Comma-separated commit IDs (empty selects published history) required: false type: string + # Select the measurement platforms without changing the driver. + platforms: + description: Comma-separated platforms, linux or macos (empty selects both) + required: false + type: string permissions: contents: read @@ -43,6 +48,7 @@ jobs: id: plan env: REVISIONS: ${{ inputs.revisions }} + PLATFORMS: ${{ inputs.platforms }} run: | set -euo pipefail revision_args=() @@ -53,6 +59,13 @@ jobs: revision_args+=(--revision="${revision}") fi done + IFS=',' read -ra platforms <<< "${PLATFORMS}" + for platform in "${platforms[@]}"; do + platform="${platform//[[:space:]]/}" + if [[ -n "${platform}" ]]; then + revision_args+=(--platform="${platform}") + fi + done matrix="$(python3 tools/benchmark_campaign.py plan --repo=. \ --history-root=benchmark-history/benchmarks --output=campaign.json "${revision_args[@]}")" echo "matrix=${matrix}" >>"${GITHUB_OUTPUT}" @@ -100,7 +113,8 @@ jobs: - name: Install reference tools for this revision run: | if [[ "${RUNNER_OS}" == Linux ]]; then - sudo apt-get update && sudo apt-get install -y findutils ripgrep fzf + # Older LLVM linkers need these Ubuntu 22.04 runtime ABIs. + sudo apt-get update && sudo apt-get install -y findutils ripgrep fzf libicu70 libxml2 else brew install findutils ripgrep fzf fi diff --git a/docs/benchmark-merged-pr-coverage.md b/docs/benchmark-merged-pr-coverage.md index dc360240c0..f7e9dd48cc 100644 --- a/docs/benchmark-merged-pr-coverage.md +++ b/docs/benchmark-merged-pr-coverage.md @@ -175,8 +175,9 @@ not the persistent automatic scheduler described above. A concurrent dispatch af the active-campaign check can still race; avoid starting multiple operators at once. The publisher can retain each complete validated platform from a failed campaign, so a -failed platform does not discard another platform's results. A targeted retry currently -remeasures both platforms. The command does not cancel a pending publisher or +failed platform does not discard another platform's results. Recovery selects the union +of missing platforms in a batch; `--pr=NUMBER` or `--batch-size=1` allows a platform-specific +retry without repeating a retained platform. The command does not cancel a pending publisher or automatically retry a failed run. ## Decisions and limits @@ -218,3 +219,43 @@ finish, and merge PR #1148. Then: 5. Freeze and identify the additional workload contract before scheduling its historical backfill. Preserve the existing simple comparisons and make compatibility explicit; a new task must not silently appear as if it had been measured in earlier reports. + +## Historical Linux linker compatibility + +The November 8 campaign reached compilation but failed before any Linux measurement: + +- [PR #986 Linux job](https://github.com/mboworks/xff/actions/runs/42498912253/job/128787591927): + historical `ld.lld` could not load `libicui18n.so.70`. +- [PR #914 Linux job](https://github.com/mboworks/xff/actions/runs/42498912253/job/128787591953): + historical `ld.lld` could not load `libxml2.so.2`. + +Both jobs used `ubuntu-latest`, which resolved to Ubuntu 26.04. New historical Linux +campaigns select `ubuntu-22.04` and explicitly install `libicu70` and `libxml2`. This +preserves each revision's compiler and LTO configuration. The selected runner remains +in the campaign plan, and validation still accepts older plans using `ubuntu-latest`. +The ordinary main benchmark workflow keeps its existing runner selection. + +The backfill dispatch also accepts `platforms=linux` or `platforms=macos`, so failed Linux +jobs can be remeasured in a new campaign without repeating completed macOS work. Earlier +plans without a platform selection still require both platforms, and the new selection +is part of the immutable campaign identity. For this campaign, retry the two original +revisions with `platforms=linux` after the fixed workflow is deployed on main. + +This pin needs a replacement before [GitHub retires Ubuntu 22.04 on April 17, 2027](https://github.com/actions/runner-images/issues/14254). +A replacement must provide the historical build dependencies, preserve build provenance, +and record the measurement environment. Replacing historical compilers would change +what the backfill measures. + +## Historical command timeouts + +The macOS #986 job completed successfully. The macOS #914 job failed at the 100,000-file +broad content-search warm-up when an invocation exceeded the fixed 60-second limit. +New CI backfill plans use a 300-second command limit, recorded in the campaign, batch, +and measurement contract. Earlier plans retain the implicit 60-second limit. Other +comparison callers keep their current default. Timeout failures still terminate the +whole worker process group and report the failed command; no timed-out sample is kept. + +After deployment, retry Linux #914/#986 together and macOS #914 separately. Preserve +the completed macOS #986 report from the original campaign. The longer command limit +has not yet been exercised on that historical macOS workload; the overall job deadline +still bounds total measurement time. diff --git a/tools/benchmark_backfill.py b/tools/benchmark_backfill.py index a5e23aa4a7..b7559d7e10 100644 --- a/tools/benchmark_backfill.py +++ b/tools/benchmark_backfill.py @@ -173,6 +173,8 @@ def make_contract(args, *, campaign=None, collection=None): raise ValueError('local prepared layouts must be broad/v2 and deep/v2 together') if args.depth < 1 or not 1 <= args.keep <= args.repetitions: raise ValueError('positive depth and 1 <= keep <= repetitions required') + if getattr(args, 'command_timeout', 60) <= 0: + raise ValueError('comparison command timeout must be positive') allocation = allocation_policy(args, cpus) repo = args.repo.resolve(strict=True) references = list(args.revision) @@ -194,6 +196,7 @@ def make_contract(args, *, campaign=None, collection=None): 'environment': environment(getattr(args, 'resource_launcher', None)), 'driver': driver_identity(), 'files': counts, 'cpus': cpus, 'depth': args.depth, 'repetitions': args.repetitions, 'retained': args.keep, + 'command_timeout_seconds': getattr(args, 'command_timeout', 60), 'require_cpu_affinity': allocation['required'], 'storage': compare.fixture_storage(args.fixture_parent, args.require_memory), 'build': {'config': 'clang_release', 'bazel': str(Path(bazel).resolve()), @@ -396,6 +399,7 @@ def run(args, batch): fixture_parent=contract['storage']['parent'], require_memory=contract['storage']['memory_required'], require_cpu_affinity=contract['require_cpu_affinity'], progress=progress, layouts=contract.get('layouts') or None, + command_timeout=contract.get('command_timeout_seconds', 60), resource_launcher=contract['environment'].get('resource_launcher', {}).get('path')) check_environment(contract['environment']) if report['tools']['xff']['sha256'] != build_record['sha256']: @@ -452,6 +456,7 @@ def main(): help='local generated layouts; hosted CI remains legacy') parser.add_argument('--repetitions', type=int, default=9) parser.add_argument('--keep', type=int, default=7) + parser.add_argument('--command-timeout', type=int, default=60, help='Maximum seconds per comparison command') parser.add_argument('--require-cpu-affinity', action='store_true') parser.add_argument('--fixture-parent', type=Path) parser.add_argument('--require-memory', action='store_true') diff --git a/tools/benchmark_campaign.py b/tools/benchmark_campaign.py index a186264b3d..be716b714b 100644 --- a/tools/benchmark_campaign.py +++ b/tools/benchmark_campaign.py @@ -16,7 +16,9 @@ import benchmark_compare as compare import benchmark_records -PLATFORMS = {'linux': ('ubuntu-latest', 'x86_64'), 'macos': ('macos-latest', 'arm64')} +# Historical LLVM 22/23 linkers require the Ubuntu 22.04 libxml2/ICU ABI. +# Keep the measurement runner pinned instead of changing historical toolchains. +PLATFORMS = {'linux': ('ubuntu-22.04', 'x86_64'), 'macos': ('macos-latest', 'arm64')} FILE_COUNTS = [*compare.DEFAULT_FILE_COUNTS, 20000, 50000, 100000] @@ -29,10 +31,12 @@ def require(condition, message): raise ValueError(message) -def make_plan(repo, history_root, references=None): +def make_plan(repo, history_root, references=None, platforms=None): tools = {'xff': {'path': 'xff'}, 'find': {}, 'rg': {}, 'fzf': {}} jobs = [] for platform, (runner, arch) in PLATFORMS.items(): + if platforms and platform not in platforms: + continue for revision in backfill.revisions(repo, references or [], None if references else history_root, platform): jobs.append({'id': platform + '-' + revision['sha'], 'platform': platform, 'os': runner, 'arch': arch, 'revision': revision}) @@ -40,9 +44,12 @@ def make_plan(repo, history_root, references=None): 'history_head': backfill.git(history_root, 'rev-parse', 'HEAD'), 'created_at': backfill.now(), 'jobs': jobs, 'contract': {'files': FILE_COUNTS, 'cpus': [1, 3], 'depth': 40, - 'repetitions': 9, 'retained': 7, 'driver': backfill.driver_identity(), + 'repetitions': 9, 'retained': 7, 'command_timeout_seconds': 300, + 'driver': backfill.driver_identity(), 'tasks': {name: sorted(participants) for name, _, participants in compare.scenarios(Path('.'), [], tools)}}} + if platforms: + plan['platforms'] = sorted(set(platforms)) plan['identity'] = identity(plan) validate_plan(plan) return plan @@ -59,6 +66,8 @@ def validate_plan(plan): and (contract['depth'], contract['repetitions'], contract['retained']) == (40, 9, 7), 'CI campaign requires the complete 1/3 grid and fastest seven of nine samples') require(contract['tasks'] and contract['driver'], 'campaign tasks and driver identities required') + timeout = contract.get('command_timeout_seconds', 60) + require(isinstance(timeout, int) and timeout > 0, 'invalid campaign command timeout') require(0 < len(plan['jobs']) <= 256, 'campaign requires 1 through 256 complete-revision jobs') ids = set() for job in plan['jobs']: @@ -66,11 +75,15 @@ def validate_plan(plan): require(re.fullmatch('[0-9a-f]{40}', sha), 'invalid historical revision') require(datetime.fromisoformat(job['revision']['date']).tzinfo is not None, 'historical commit date requires a timezone') - require(platform in PLATFORMS and (job['os'], job['arch']) == PLATFORMS[platform], + runners = {'ubuntu-latest', 'ubuntu-22.04'} if platform == 'linux' else {'macos-latest'} + require(platform in PLATFORMS and job['os'] in runners and job['arch'] == PLATFORMS[platform][1], 'invalid campaign platform') require(job['id'] == platform + '-' + sha and job['id'] not in ids, 'duplicate or invalid campaign job') ids.add(job['id']) - require({job['platform'] for job in plan['jobs']} == set(PLATFORMS), 'both CI platforms are required') + requested = plan.get('platforms', list(PLATFORMS)) + require(isinstance(requested, list) and requested and set(requested) <= set(PLATFORMS), + 'invalid requested campaign platforms') + require({job['platform'] for job in plan['jobs']} == set(requested), 'all requested CI platforms are required') def run_job(plan, job_id, repo, output, disk_cache, fixture_parent, resource_launcher=None): @@ -87,6 +100,7 @@ def run_job(plan, job_id, repo, output, disk_cache, fixture_parent, resource_lau revision=[job['revision']['sha']], revisions_file=None, history_root=None, history_platform=None, files=contract['files'], cpus=contract['cpus'], depth=contract['depth'], repetitions=contract['repetitions'], keep=contract['retained'], + command_timeout=contract.get('command_timeout_seconds', 60), require_cpu_affinity=job['platform'] == 'linux', require_memory=job['platform'] == 'linux', fixture_parent=fixture_parent, disk_cache=disk_cache, resource_launcher=resource_launcher) collection = {'id': int(os.environ['GITHUB_RUN_ID']), 'run_attempt': int(os.environ['GITHUB_RUN_ATTEMPT']), @@ -99,6 +113,8 @@ def run_job(plan, job_id, repo, output, disk_cache, fixture_parent, resource_lau def validate_report(record, batch, job, plan): contract, expected = batch['contract'], plan['contract'] + require(contract.get('command_timeout_seconds', 60) == expected.get('command_timeout_seconds', 60), + 'batch command timeout does not match campaign') require(batch['identity'] == identity(contract), 'batch identity changed') require(contract['campaign'] == plan['identity'], 'batch belongs to another campaign') require(contract['purpose'] == 'ci-replacement', 'local series cannot replace CI history') @@ -115,6 +131,8 @@ def validate_measurements(record, batch, revision, tasks): """Validate observations shared by CI replacement and independent local series.""" contract = batch['contract'] expected = contract + require(record['tool_comparisons']['contract'].get('command_timeout_seconds', 60) + == expected.get('command_timeout_seconds', 60), 'comparison command timeout changed') arch = contract['environment']['machine'].lower() require(record['schema'] == 1 and record['kind'] == 'backfill', 'expected a backfill report') for key in ('platform', 'series', 'purpose', 'replacement_target', 'allocation'): @@ -246,6 +264,8 @@ def main(): for name in ('repo', 'history-root', 'output'): plan.add_argument('--' + name, type=Path, required=True) plan.add_argument('--revision', action='append', default=[], help='Measure this revision instead of site history') + plan.add_argument('--platform', choices=tuple(PLATFORMS), action='append', default=[], + help='Measure only selected platforms (default: both)') run = modes.add_parser('run') for name in ('plan', 'repo', 'output', 'fixture-parent'): run.add_argument('--' + name, type=Path, required=True) @@ -259,7 +279,7 @@ def main(): publish.add_argument('--repository', required=True) args = parser.parse_args() if args.mode == 'plan': - result = make_plan(args.repo, args.history_root, args.revision) + result = make_plan(args.repo, args.history_root, args.revision, args.platform) backfill.write_json(args.output, result) print(json.dumps({'include': [{key: job[key] for key in ('id', 'os', 'platform', 'arch')} for job in result['jobs']]})) diff --git a/tools/benchmark_campaign_test.py b/tools/benchmark_campaign_test.py index d52bbe0bea..9a59a4e563 100644 --- a/tools/benchmark_campaign_test.py +++ b/tools/benchmark_campaign_test.py @@ -146,6 +146,25 @@ def test_planner_uses_explicit_revisions_instead_of_published_history(self): self.assertEqual([call.args[1:] for call in revisions.call_args_list], [([HEAD], None, 'linux'), ([HEAD], None, 'macos')]) + def test_historical_linux_runner_is_pinned_and_earlier_plans_remain_valid(self): + self.assertEqual(campaign.PLATFORMS['linux'], ('ubuntu-22.04', 'x86_64')) + selected = plan() + for job in selected['jobs']: + if job['platform'] == 'linux': + job['os'] = 'ubuntu-latest' + selected['identity'] = campaign.identity({key: value for key, value in selected.items() if key != 'identity'}) + campaign.validate_plan(selected) + + def test_explicit_linux_only_retry_does_not_schedule_macos(self): + revision = plan()['jobs'][0]['revision'] + with mock.patch.object(campaign.backfill, 'revisions', return_value=[revision]), \ + mock.patch.object(campaign.backfill, 'git', side_effect=[DRIVER, 'c' * 40]): + selected = campaign.make_plan(Path('/repo'), Path('/history'), [HEAD], ['linux']) + self.assertEqual(selected['platforms'], ['linux']) + self.assertEqual([(job['platform'], job['os']) for job in selected['jobs']], + [('linux', 'ubuntu-22.04')]) + campaign.validate_plan(selected) + def test_plan_rejects_mutation_duplicates_partial_grid_and_missing_platform(self): mutations = [lambda value: value['jobs'].append(value['jobs'][0]), lambda value: value['jobs'].pop(), diff --git a/tools/benchmark_compare.py b/tools/benchmark_compare.py index fad272d5da..76fef402de 100644 --- a/tools/benchmark_compare.py +++ b/tools/benchmark_compare.py @@ -53,17 +53,20 @@ def measure(spec): def invoke(spec, worker): + timeout = spec.get('command_timeout_seconds', 60) + if not isinstance(timeout, (int, float)) or not math.isfinite(timeout) or timeout <= 0: + raise ValueError('comparison timeout must be finite and positive') with tempfile.NamedTemporaryFile(mode="w", suffix=".json") as request: json.dump(spec, request) request.flush() child = subprocess.Popen([*worker, "--worker", request.name], stdout=subprocess.PIPE, stderr=subprocess.PIPE, start_new_session=True) try: - stdout, stderr = child.communicate(timeout=60) + stdout, stderr = child.communicate(timeout=timeout) except subprocess.TimeoutExpired: os.killpg(child.pid, signal.SIGKILL) child.communicate() - raise ValueError("comparison command exceeded 60 seconds") from None + raise ValueError(f"comparison command exceeded {timeout} seconds: {spec.get('pipeline', [])}") from None if child.returncode: raise ValueError(f"comparison worker failed: {stderr.decode(errors='replace')}") return json.loads(stdout) @@ -195,7 +198,8 @@ def cpu_allocation(cpus, require_cpu_affinity=False): def collect(binary, files=2000, depth=40, repetitions=9, worker=None, require_tools=False, fixture_parent=None, require_memory=False, cpus=1, require_cpu_affinity=False, keep=None, fixtures=None, progress=None, - shapes=("broad", "deep"), round_offset=0, prepared_fixtures=None, resource_launcher=None): + shapes=("broad", "deep"), round_offset=0, prepared_fixtures=None, resource_launcher=None, + command_timeout=60): progress = progress or MeasurementProgress() keep = min(7, repetitions) if keep is None else keep if not 1 <= keep <= repetitions: @@ -285,12 +289,15 @@ def collect(binary, files=2000, depth=40, repetitions=9, worker=None, require_to f'{shape}/{name}; {label}; {phase}') sample = invoke({"pipeline": entry["pipeline"], "stdin": entry["stdin"], "cwd": entry["cwd"], "environment": environment, "cpu_affinity": affinity, - "resource_launcher": accounting["path"]}, worker) + "resource_launcher": accounting["path"], + "command_timeout_seconds": command_timeout}, worker) validate_output(sample, expected, entry["pipeline"]) completed += 1 if repetition: entry["samples"].append(sample) progress(f'Workers: {cpus} Files: {files:,} Completed: {completed}/{total}; {shape}/{name}') + if command_timeout != 60: + report['contract']['command_timeout_seconds'] = command_timeout for tool in tools.values(): if tool["status"] == "available" and digest(tool["path"]) != tool["sha256"]: raise ValueError("tool binary changed during measurement") @@ -301,7 +308,8 @@ def collect(binary, files=2000, depth=40, repetitions=9, worker=None, require_to def collect_scales(binary, file_counts, depth=40, repetitions=9, require_tools=False, fixture_parent=None, require_memory=False, cpu_counts=(1, 4), require_cpu_affinity=False, keep=None, fixtures=None, - shard_plan=None, shard_index=0, progress=None, layouts=None, resource_launcher=None): + shard_plan=None, shard_index=0, progress=None, layouts=None, resource_launcher=None, + command_timeout=60): """Retain independent scales without mixing sample populations or tool identities.""" if not file_counts or len(set(file_counts)) != len(file_counts) or min(file_counts) < 1 or depth < 1: raise ValueError("file counts must be unique and positive; depth must be positive") @@ -361,7 +369,7 @@ def collect_scales(binary, file_counts, depth=40, repetitions=9, require_tools=F round_offset=shard_index * sampling[0] if sampling else 0, fixtures={shape: value for (shape, count), value in fixtures.items() if count == files} if fixtures is not None else None, prepared_fixtures=prepared_fixtures, - resource_launcher=resource_launcher) + resource_launcher=resource_launcher, command_timeout=command_timeout) completed_scales += 1 progress(f'Workers: {cpus} Files: {files:,} Completed scale: {completed_scales}/{total_scales}', force=True) for task in report["tasks"]: diff --git a/tools/benchmark_compare_test.py b/tools/benchmark_compare_test.py index 322936b30b..f3cc4b1b8f 100644 --- a/tools/benchmark_compare_test.py +++ b/tools/benchmark_compare_test.py @@ -82,6 +82,16 @@ def test_timeout_terminates_the_whole_worker_group(self): compare.invoke({}, ["worker"]) kill.assert_called_once_with(12345, signal.SIGKILL) + def test_configured_timeout_is_applied_and_reported(self): + child = mock.Mock(pid=12345) + child.communicate.side_effect = [subprocess.TimeoutExpired('worker', 300), (b'', b'')] + with mock.patch.object(compare.subprocess, 'Popen', return_value=child), \ + mock.patch.object(compare.os, 'killpg') as kill: + with self.assertRaisesRegex(ValueError, '300 seconds'): + compare.invoke({'command_timeout_seconds': 300}, ['worker']) + self.assertEqual(child.communicate.call_args_list[0].kwargs, {'timeout': 300}) + kill.assert_called_once_with(12345, signal.SIGKILL) + def test_manifest_and_task_populations(self): with tempfile.TemporaryDirectory() as temporary: root = Path(temporary) / 'root' diff --git a/tools/benchmark_recovery.py b/tools/benchmark_recovery.py index 7331383755..ef8e2ab0d0 100644 --- a/tools/benchmark_recovery.py +++ b/tools/benchmark_recovery.py @@ -143,10 +143,13 @@ def main(): if any(run['status'] != 'completed' for run in campaigns): raise ValueError('a backfill campaign is already active or queued; retry after it finishes') selected = candidates[:args.batch_size] + platforms = sorted({platform for row in selected for platform in row['missing']}) # The deployed workflow must support the revisions input added by PR #1147. subprocess.run(['gh', 'workflow', 'run', 'benchmark_backfill.yml', '--repo', api.repository, - '--ref', 'main', '-f', 'revisions=' + ','.join(row['commit'] for row in selected)], check=True) - print('Dispatched both platforms for ' + ', '.join(f"PR #{row['pr']}" for row in selected) + '.', flush=True) + '--ref', 'main', '-f', 'revisions=' + ','.join(row['commit'] for row in selected), + '-f', 'platforms=' + ','.join(platforms)], check=True) + print('Dispatched ' + ', '.join(platforms) + ' for ' + + ', '.join(f"PR #{row['pr']}" for row in selected) + '.', flush=True) if __name__ == '__main__': diff --git a/tools/benchmark_recovery_test.py b/tools/benchmark_recovery_test.py index 69d3939d21..2434643b7c 100644 --- a/tools/benchmark_recovery_test.py +++ b/tools/benchmark_recovery_test.py @@ -89,7 +89,7 @@ def test_dispatch_refuses_active_campaign(self): def test_dispatch_uses_the_targeted_main_workflow(self): api = FakeGitHub() - selected = {'state': 'needs backfill', 'pr': 7, 'commit': SHA} + selected = {'state': 'needs backfill', 'pr': 7, 'commit': SHA, 'missing': ['linux']} with mock.patch.object(recovery.benchmark_preview_ci, 'GitHub', return_value=api), \ mock.patch.object(recovery, 'inventory', return_value=[selected]), \ mock.patch.object(api, 'pages', return_value=[]), \ @@ -97,7 +97,8 @@ def test_dispatch_uses_the_targeted_main_workflow(self): mock.patch('sys.argv', ['benchmark_recovery', '--repository', 'owner/repo', '--dispatch-next']): recovery.main() run.assert_called_once_with(['gh', 'workflow', 'run', 'benchmark_backfill.yml', '--repo', - 'owner/repo', '--ref', 'main', '-f', 'revisions=' + SHA], check=True) + 'owner/repo', '--ref', 'main', '-f', 'revisions=' + SHA, + '-f', 'platforms=linux'], check=True) if __name__ == '__main__':