Repository navigation
[experiment] ci: benchmark the longest PR checks on 16/32/64-core runners - #343095
Draft
Michael Lively (Yoyokrazy) wants to merge 4 commits into
Draft
Michael Lively (Yoyokrazy) wants to merge 4 commits into
Michael Lively (Yoyokrazy) wants to merge 4 commits into
Conversation
Temporary experiment for microsoft/vscode-engineering#4273, do not merge. - Add `runner` and `benchmark` inputs to pr-linux-test.yml and pr-win32-test.yml. `runner` defaults to today's runner. `benchmark` samples CPU and memory in the background and suffixes artifact names. - Add benchmark jobs to pr.yml that run Linux and Windows Electron, Electron-Unit, Remote, and both smoke shards unchanged on the 16, 32, and 64-core GitHub-hosted pools, next to the original jobs. - Run Compile & Hygiene as a matrix: the first entry is the unchanged required check; the others run on 16/32/64 core with ESLint after the parallel checks, inside them, or inside them with one worker per core. - Let VSCODE_ESLINT_CONCURRENCY override ESLint's `concurrency` option. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The temporary runner-size benchmark jobs call the PR test workflows but are not required checks. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
One ESLint worker per core ran out of memory on the 16 and 64-core runners. Benchmark a fixed cap of 8 workers on 32 and 64 cores instead. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Warning
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Copilot review overview
7 open issues
Start-Processand not tracked/stopped before the artifact… · NewStart-Processand not tracked/stopped before the artifact… · Newname(e.g. both Electron and Electron-Unit show… · Newname(e.g. both Electron and Electron-Unit show… · New' AUTO ','16\\n', or… · New
What changed in this PR
This PR adds temporary CI benchmark jobs to measure how long-running PR checks perform on larger GitHub-hosted runners, and introduces an environment-variable override for ESLint concurrency to support those experiments.
Changes:
- Adds
runner/benchmarkinputs to the Linux/Windows reusable test workflows, including resource sampling and benchmark-safe artifact naming. - Converts “Compile & Hygiene” into a matrix to test runner sizes and multiple ESLint-parallelization strategies.
- Adds
VSCODE_ESLINT_CONCURRENCYparsing in build tooling plus unit tests, and updates required-check detection tests to ignorebench-*jobs.
| File | Description |
|---|---|
| build/eslint.ts | Adds VSCODE_ESLINT_CONCURRENCY parsing and wires it into ESLint invocation. |
| build/lib/test/eslint.test.ts | Adds coverage for the new concurrency parsing helper. |
| build/lib/test/detectChanges.test.ts | Excludes benchmark jobs from required-check comparison logic. |
| .github/workflows/pr.yml | Adds bench job matrices + resource sampling; expands Compile & Hygiene into a matrix. |
| .github/workflows/pr-linux-test.yml | Adds runner selection, benchmark mode, artifact name disambiguation, and resource sampling. |
| .github/workflows/pr-win32-test.yml | Adds runner selection, benchmark mode, artifact name disambiguation, and resource sampling. |
Review effort: Lite
Comment on lines
+58
to
+63
| - name: Start resource sampler | ||
| if: ${{ inputs.benchmark }} | ||
| run: | | ||
| mkdir -p .build/resources | ||
| { nproc; free -m; df -h "$GITHUB_WORKSPACE" /tmp; lscpu; } > .build/resources/system.txt 2>&1 | ||
| nohup vmstat -n -t 5 > .build/resources/vmstat.log 2>&1 & |
Comment on lines
+509
to
+516
| - name: Publish resource samples | ||
| uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 | ||
| if: ${{ always() && inputs.benchmark }} | ||
| continue-on-error: true | ||
| with: | ||
| name: ${{ format('resources-linux-{0}-{1}-{2}', env.VSCODE_ARCH, env.ARTIFACT_NAME, github.run_attempt) }} | ||
| path: .build/resources | ||
| if-no-files-found: ignore |
Comment on lines
+80
to
+81
| Start-Process pwsh -WindowStyle Hidden -ArgumentList '-NoProfile', '-File', "`"$sampler`"", '-Output', "`"$(Join-Path $directory counters.csv)`"" | ||
|
|
Comment on lines
+394
to
+401
| - name: Publish resource samples | ||
| uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 | ||
| if: ${{ always() && inputs.benchmark }} | ||
| continue-on-error: true | ||
| with: | ||
| name: ${{ format('resources-windows-{0}-{1}-{2}', env.VSCODE_ARCH, env.ARTIFACT_NAME, github.run_attempt) }} | ||
| path: .build/resources | ||
| if-no-files-found: ignore |
Comment on lines
+421
to
+430
| bench-linux-electron-unit-tests: | ||
| name: Bench Linux ${{ matrix.size }}-core | ||
| strategy: | ||
| fail-fast: false | ||
| matrix: | ||
| size: [16, 32, 64] | ||
| uses: ./.github/workflows/pr-linux-test.yml | ||
| with: | ||
| job_name: Electron-Unit | ||
| runner: vscode-large-runners-ubuntu-${{ matrix.size }}-core |
Comment on lines
+436
to
+445
| bench-linux-electron-tests: | ||
| name: Bench Linux ${{ matrix.size }}-core | ||
| strategy: | ||
| fail-fast: false | ||
| matrix: | ||
| size: [16, 32, 64] | ||
| uses: ./.github/workflows/pr-linux-test.yml | ||
| with: | ||
| job_name: Electron | ||
| runner: vscode-large-runners-ubuntu-${{ matrix.size }}-core |
Comment on lines
+26
to
+34
| if (!value || value === 'auto') { | ||
| return 'auto'; | ||
| } | ||
| if (value === 'off') { | ||
| return 'off'; | ||
| } | ||
| const workers = Number(value); | ||
| if (!Number.isInteger(workers) || workers < 1) { | ||
| throw new Error(`Invalid VSCODE_ESLINT_CONCURRENCY: ${value}`); |
The Windows benchmark jobs are disk-bound, so record what the runners' disks and real-time scanning look like. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
Screenshot ChangesBase: Changed (1)
|
| Before | After |
|---|---|
 |
 |
Collaborator
Author
Results: longest PR check jobs on 16 / 32 / 64-core runners (size only, no test changes)Draft PR: #343095 · tracking: microsoft/vscode-engineering#4273 Short answer:
Headline: median job time, minutes (node_modules cache hit)
¹ 8-core numbers come from the production-pool PR #343094: 1–2 runs per job, so they show the trend only. ² On 16 cores Samples: 4 per job × size (run A plus 3 attempts of run C), all on one commit per run. Every run also ran the unchanged original jobs, which give the in-run baseline ("Today"). The in-run baseline matches the Oct 20 medians (Compile 8.0 vs 8.0; Windows / Electron 32.1 vs 25.5–37.5 depending on Agent Host E2E). This PR changes WhyBusy cores during the main steps, from the
Recommendation
Related work:
Cache, queue, flakes
Cost of the experimentTwo full workflow runs plus two full re-runs (run A; run C × 3 attempts). Two more runs were cancelled within ~7 min: one to fix a build-script test that enumerates Full per-step tables (median, min–max; minutes)"Total, cache hit" counts only successful jobs whose node_modules cache hit. Step times include runs where the step failed on a test assertion; the suites still ran to completion. Compile & Hygiene step times come from successful runs only. Windows
Linux
Compile & Hygiene (Linux)
Resource usage per step (median across runs)
Data: runs 39522774727 and 39528120074 (attempts 1–3). 8-core: 39522227766 and 39539559733. |
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Warning
Experiment, do not merge. The temporary benchmark jobs make this PR non-mergeable by design. Some of them may fail on the new runners (for example, WSL on Windows smoke shard 1). That's expected and is part of what gets measured.
Part of https://github.com/microsoft/vscode-engineering/issues/4273.
What this measures
How much faster the longest-running PR check jobs get on bigger GitHub-hosted runners with no test-code changes. Only
runs-onchanges, plus ESLint settings in a step that already runs in parallel. Every size runs on the same commit in the same workflow run as the original jobs, so each run also gives an in-run baseline.ubuntu-24.04(4-core)vscode-large-runners-ubuntu-{16,32,64}-corevscode-large-runners-windows-{16,32,64}-core(Server 2025)vscode-large-runners-ubuntu-{16,32,64}-core× 3 ESLint variantsThe 8-core pools are production pools, so this PR doesn't use them. The 8-core data point will come from the PR that moves the PR checks onto them.
Changes
pr-linux-test.ymlandpr-win32-test.ymlget two new inputs:runner: defaults to today's runner (ubuntu-24.04/ the 1ES Windows pool).benchmark: samples CPU and memory in the background (vmstaton Linux,Get-Counteron Windows), uploads the samples as aresources-*artifact, and adds the runner label to artifact names so they don't collide with the original jobs.pr.yml:bench-*jobs run the jobs above on 16/32/64 core. They're not part ofrequired-checks.after: after thenpm-run-all2 -lpgroup, as today.parallel: inside the group. It was moved out because the 32 GB machine ran out of memory (ci: run ESLint after parallel compile checks #339153).parallel-8-workers(32/64 core): inside the group, with at most 8 ESLint workers. Run A used one worker per core instead (parallel-all-cores). That ran out of memory on 16 and 64 cores, so it was replaced.build/eslint.ts:VSCODE_ESLINT_CONCURRENCY(auto/off/N) overrides ESLint'sconcurrencyoption. The default (auto) is unchanged.build/lib/test/detectChanges.test.ts: the temporarybench-*jobs are excluded from the check that every PR test job reports throughrequired-checks.Pool usage per run: up to 8 concurrent jobs on each Linux pool, 5 on each Windows pool.
Not in this PR
--jobs/--storage. The runner already runsmin(6 suites, cores)jobs, so--jobshas no headroom on 16+ cores.--storage tmpfswould drop the disk-backed lifecycle pass, which changes coverage. Because this PR changes.github/workflows/pr*andbuild/, every Electron job runs the full Agent Host E2E profile.Results
Results comment, also posted on vscode-engineering#4273:
auto(32 workers) runs out of memory on 64-core.npm ciare slower than on 1ES. The cause is I/O, not CPU, and Defender is off.Validation
actionlinton the three workflows: no new findings. The only warnings left are the existing unknown1ES.Pool=…labels.test-build-scripts(including the neweslintand the updateddetectChangestests) andType check /build/ scriptsin every Compile & Hygiene job. Both passed.