feat: Resolution time benchmark github action#2239
feat: Resolution time benchmark github action#2239cieplypolar wants to merge 36 commits intomainfrom
Conversation
|
pkg.pr.new packages benchmark commit |
📊 Bundle Size Comparison
👀 Notable resultsStatic test results:No major changes. Dynamic test results:No major changes. 📋 All resultsClick to reveal the results table (349 entries).
If you wish to run a comparison for other, slower bundlers, run the 'Tree-shake test' from the GitHub Actions menu. |
…GPU into feat/proc-gen-func
Resolution Time Benchmark---
config:
themeVariables:
xyChart:
plotColorPalette: "#E63946, #3B82F6, #059669"
---
xychart
title "Random Branching (🔴 PR | 🔵 main | 🟢 release)"
x-axis "max depth" [1, 2, 3, 4, 5, 6, 7, 8]
y-axis "time (ms)"
line [0.87, 1.87, 4.19, 6.10, 6.41, 12.02, 21.48, 23.73]
line [0.79, 2.22, 3.57, 9.89, 1.79, 8.20, 13.50, 41.13]
line [0.49, 1.33, 4.34, 9.41, 1.80, 8.06, 12.96, 39.59]
---
config:
themeVariables:
xyChart:
plotColorPalette: "#E63946, #3B82F6, #059669"
---
xychart
title "Linear Recursion (🔴 PR | 🔵 main | 🟢 release)"
x-axis "max depth" [1, 2, 3, 4, 5, 6, 7, 8]
y-axis "time (ms)"
line [0.29, 0.51, 0.68, 0.86, 1.13, 1.19, 1.41, 1.54]
line [0.28, 0.52, 0.61, 0.71, 1.44, 1.03, 1.70, 1.74]
line [0.32, 0.63, 0.65, 1.06, 1.93, 0.97, 1.80, 1.79]
---
config:
themeVariables:
xyChart:
plotColorPalette: "#E63946, #3B82F6, #059669"
---
xychart
title "Full Tree (🔴 PR | 🔵 main | 🟢 release)"
x-axis "max depth" [1, 2, 3, 4, 5, 6, 7, 8]
y-axis "time (ms)"
line [0.87, 1.90, 4.07, 5.88, 11.64, 24.28, 54.24, 111.67]
line [1.44, 2.66, 7.25, 10.33, 19.13, 25.85, 58.40, 107.65]
line [0.99, 2.53, 7.06, 7.27, 16.57, 25.07, 55.01, 100.24]
|
There was a problem hiding this comment.
Pull request overview
Adds CI automation to benchmark TypeGPU function resolution time on PRs and compare results against the PR base branch and the release branch, posting Mermaid charts back to the PR for visibility into performance regressions.
Changes:
- Introduces a new GitHub Actions workflow to run the resolution-time benchmark across three checkouts (PR/base/release) and post comparison charts to the PR.
- Increases benchmark sampling in
procedural.tsand adjusts output selection logic intended to reduce warmup/JIT bias.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 8 comments.
| File | Description |
|---|---|
| apps/resolution-time/procedural.ts | Increases sample count and modifies how benchmark results are collected/trimmed before writing JSON outputs. |
| .github/workflows/resolution-time.yml | New CI workflow to execute benchmarks on multiple branches, generate Mermaid charts, and post/update a PR comment with results. |
Comments suppressed due to low confidence (1)
apps/resolution-time/procedural.ts:267
runBenchmarkdoublessamplesbut still pushes all iterations intooutput. Later you try to discard the first half as warmup viaresults.slice(...), but sinceresultsaggregates runs across multiplemaxDepthvalues, this ends up discarding entire earlier depths rather than the warmup iterations for each depth. Consider handling warmup insiderunBenchmark(e.g., run 2× but only push the latter half for that specificmaxDepth, or use a local array and append the filtered subset).
function runBenchmark(input: ProcGenConfig, output: BenchmarkResult[]) {
Object.assign(config, { samples: (input.samples ?? SAMPLES) * 2 }, input);
branchingUnrollArray = getArrayForUnroll(config.branching);
for (let i = 0; i < config.samples; i++) {
rand = splitmix32(config.seed);
const result = benchmarkResolve();
output.push(result);
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
06a64a1 to
3564492
Compare
…/TypeGPU into feat/resolution-time-ci
| - name: Copy resolution-time app to release checkout | ||
| run: | | ||
| if [ ! -d release-branch/apps/resolution-time ]; then | ||
| cp -r target-branch/apps/resolution-time release-branch/apps/resolution-time | ||
| echo "Copied resolution-time app from target branch to release checkout" | ||
| else | ||
| echo "resolution-time app already exists in release checkout" | ||
| fi |
There was a problem hiding this comment.
So, we only copy the folder if it's missing, right? Will this step be removed after next release?
There was a problem hiding this comment.
yes, I included this if statement in case I forget about it 👻
|
Also, out of curiousity, do we have access to statistics about how many minutes does each action take on average/in total? |
Take a look |
Changes:
Example comparison chart is below in the comments section.
procedural.tsin this PR, the charts might not align, but everything should be fine after the mergeTODO:
std.rangeintgpu.unrollwhen merged to mainresolution-timeapp when merged toreleasebranchincluded check if app exists on release