Skip to content

Commit d2ddf94

Browse files
authored
Include graph runtime benchmarks in CI perf regression runs (#2780)
* Include graph runtime benchmarks in ci regression run * Update benchmarking workflow * Add render slowdown * Fix baseline cache * Remove benchmark compilation validation run * Include render node in runtime benchmarks * Collapse sections without changes * Readd rulers between the sections * Add review suggestions * Rulers rule * Fix whitespace
1 parent b491cfc commit d2ddf94

File tree

9 files changed

+180
-91
lines changed

9 files changed

+180
-91
lines changed

.github/workflows/comment-profiling-changes.yaml

Lines changed: 149 additions & 64 deletions
Original file line numberDiff line numberDiff line change
@@ -10,31 +10,33 @@ jobs:
1010
profile:
1111
runs-on: ubuntu-latest
1212
steps:
13-
- uses: actions/checkout@v3
13+
- uses: actions/checkout@v4
1414
with:
1515
fetch-depth: 0
1616

1717
- name: Install Rust
18-
uses: actions-rs/toolchain@v1
19-
with:
20-
profile: minimal
21-
toolchain: stable
18+
uses: dtolnay/rust-toolchain@stable
2219

2320
- name: Install Valgrind
2421
run: |
2522
sudo apt update
2623
sudo apt install -y valgrind
2724
28-
- name: Cache dependencies
29-
uses: actions/cache@v3
25+
- name: Cache Rust dependencies
26+
uses: Swatinem/rust-cache@v2
27+
with:
28+
# Cache on Cargo.lock file
29+
cache-on-failure: true
30+
31+
- name: Cache iai-callgrind binary
32+
id: cache-iai
33+
uses: actions/cache@v4
3034
with:
31-
path: |
32-
~/.cargo/registry
33-
~/.cargo/git
34-
target
35-
key: ${{ runner.os }}-cargo-${{ hashFiles('**/Cargo.lock') }}
35+
path: ~/.cargo/bin/iai-callgrind-runner
36+
key: ${{ runner.os }}-iai-callgrind-runner-0.12.3
3637

3738
- name: Install iai-callgrind
39+
if: steps.cache-iai.outputs.cache-hit != 'true'
3840
run: |
3941
cargo install [email protected]
4042
@@ -43,9 +45,29 @@ jobs:
4345
git fetch origin master:master
4446
git checkout master
4547
48+
- name: Get master commit SHA
49+
id: master-sha
50+
run: echo "sha=$(git rev-parse HEAD)" >> $GITHUB_OUTPUT
51+
52+
- name: Cache benchmark baselines
53+
id: cache-benchmark-baselines
54+
uses: actions/cache@v4
55+
with:
56+
path: target/iai
57+
key: ${{ runner.os }}-benchmark-baselines-master-${{ steps.master-sha.outputs.sha }}
58+
restore-keys: |
59+
${{ runner.os }}-benchmark-baselines-master-
60+
4661
- name: Run baseline benchmarks
62+
if: steps.cache-benchmark-baselines.outputs.cache-hit != 'true'
4763
run: |
64+
# Compile benchmarks
4865
cargo bench --bench compile_demo_art_iai -- --save-baseline=master
66+
67+
# Runtime benchmarks
68+
cargo bench --bench update_executor_iai -- --save-baseline=master
69+
cargo bench --bench run_once_iai -- --save-baseline=master
70+
cargo bench --bench run_cached_iai -- --save-baseline=master
4971
5072
- name: Checkout PR branch
5173
run: |
@@ -54,13 +76,33 @@ jobs:
5476
- name: Run PR benchmarks
5577
id: benchmark
5678
run: |
57-
BENCH_OUTPUT=$(cargo bench --bench compile_demo_art_iai -- --baseline=master --output-format=json | jq -sc | sed 's/\\"//g')
58-
echo "BENCHMARK_OUTPUT<<EOF" >> $GITHUB_OUTPUT
59-
echo "$BENCH_OUTPUT" >> $GITHUB_OUTPUT
79+
# Compile benchmarks
80+
COMPILE_OUTPUT=$(cargo bench --bench compile_demo_art_iai -- --baseline=master --output-format=json | jq -sc | sed 's/\\"//g')
81+
82+
# Runtime benchmarks
83+
UPDATE_OUTPUT=$(cargo bench --bench update_executor_iai -- --baseline=master --output-format=json | jq -sc | sed 's/\\"//g')
84+
RUN_ONCE_OUTPUT=$(cargo bench --bench run_once_iai -- --baseline=master --output-format=json | jq -sc | sed 's/\\"//g')
85+
RUN_CACHED_OUTPUT=$(cargo bench --bench run_cached_iai -- --baseline=master --output-format=json | jq -sc | sed 's/\\"//g')
86+
87+
# Store outputs
88+
echo "COMPILE_OUTPUT<<EOF" >> $GITHUB_OUTPUT
89+
echo "$COMPILE_OUTPUT" >> $GITHUB_OUTPUT
90+
echo "EOF" >> $GITHUB_OUTPUT
91+
92+
echo "UPDATE_OUTPUT<<EOF" >> $GITHUB_OUTPUT
93+
echo "$UPDATE_OUTPUT" >> $GITHUB_OUTPUT
94+
echo "EOF" >> $GITHUB_OUTPUT
95+
96+
echo "RUN_ONCE_OUTPUT<<EOF" >> $GITHUB_OUTPUT
97+
echo "$RUN_ONCE_OUTPUT" >> $GITHUB_OUTPUT
98+
echo "EOF" >> $GITHUB_OUTPUT
99+
100+
echo "RUN_CACHED_OUTPUT<<EOF" >> $GITHUB_OUTPUT
101+
echo "$RUN_CACHED_OUTPUT" >> $GITHUB_OUTPUT
60102
echo "EOF" >> $GITHUB_OUTPUT
61103
62104
- name: Make old comments collapsed by default
63-
uses: actions/github-script@v6
105+
uses: actions/github-script@v7
64106
with:
65107
github-token: ${{secrets.GITHUB_TOKEN}}
66108
script: |
@@ -85,11 +127,15 @@ jobs:
85127
}
86128
87129
- name: Comment PR
88-
uses: actions/github-script@v6
130+
uses: actions/github-script@v7
89131
with:
90132
github-token: ${{secrets.GITHUB_TOKEN}}
91133
script: |
92-
const benchmarkOutput = JSON.parse(`${{ steps.benchmark.outputs.BENCHMARK_OUTPUT }}`);
134+
const compileOutput = JSON.parse(`${{ steps.benchmark.outputs.COMPILE_OUTPUT }}`);
135+
const updateOutput = JSON.parse(`${{ steps.benchmark.outputs.UPDATE_OUTPUT }}`);
136+
const runOnceOutput = JSON.parse(`${{ steps.benchmark.outputs.RUN_ONCE_OUTPUT }}`);
137+
const runCachedOutput = JSON.parse(`${{ steps.benchmark.outputs.RUN_CACHED_OUTPUT }}`);
138+
93139
let significantChanges = false;
94140
let commentBody = "";
95141
@@ -110,58 +156,97 @@ jobs:
110156
return str.padStart(len);
111157
}
112158
113-
for (const benchmark of benchmarkOutput) {
114-
if (benchmark.callgrind_summary && benchmark.callgrind_summary.summaries) {
115-
const summary = benchmark.callgrind_summary.summaries[0];
116-
const irDiff = summary.events.Ir;
117-
118-
if (irDiff.diff_pct !== null) {
119-
const changePercentage = formatPercentage(irDiff.diff_pct);
120-
const color = irDiff.diff_pct > 0 ? "red" : "lime";
121-
122-
commentBody += "---\n\n";
123-
commentBody += `${benchmark.module_path} ${benchmark.id}:${benchmark.details}\n`;
124-
commentBody += `Instructions: \`${formatNumber(irDiff.old)}\` (master) -> \`${formatNumber(irDiff.new)}\` (HEAD) : `;
125-
commentBody += `$$\\color{${color}}${changePercentage.replace("%", "\\\\%")}$$\n\n`;
159+
function processBenchmarkOutput(benchmarkOutput, sectionTitle, isLast = false) {
160+
let sectionBody = "";
161+
let hasResults = false;
162+
let hasSignificantChanges = false;
163+
164+
for (const benchmark of benchmarkOutput) {
165+
if (benchmark.callgrind_summary && benchmark.callgrind_summary.summaries) {
166+
const summary = benchmark.callgrind_summary.summaries[0];
167+
const irDiff = summary.events.Ir;
126168
127-
commentBody += "<details>\n<summary>Detailed metrics</summary>\n\n```\n";
128-
commentBody += `Baselines: master| HEAD\n`;
129-
130-
for (const [eventKind, costsDiff] of Object.entries(summary.events)) {
131-
if (costsDiff.diff_pct !== null) {
132-
const changePercentage = formatPercentage(costsDiff.diff_pct);
133-
const line = `${padRight(eventKind, 20)} ${padLeft(formatNumber(costsDiff.old), 11)}|${padLeft(formatNumber(costsDiff.new), 11)} ${padLeft(changePercentage, 15)}`;
134-
commentBody += `${line}\n`;
169+
if (irDiff.diff_pct !== null) {
170+
hasResults = true;
171+
const changePercentage = formatPercentage(irDiff.diff_pct);
172+
const color = irDiff.diff_pct > 0 ? "red" : "lime";
173+
174+
sectionBody += `**${benchmark.module_path} ${benchmark.id}:${benchmark.details}**\n`;
175+
sectionBody += `Instructions: \`${formatNumber(irDiff.old)}\` (master) → \`${formatNumber(irDiff.new)}\` (HEAD) : `;
176+
sectionBody += `$$\\color{${color}}${changePercentage.replace("%", "\\\\%")}$$\n\n`;
177+
178+
sectionBody += "<details>\n<summary>Detailed metrics</summary>\n\n```\n";
179+
sectionBody += `Baselines: master| HEAD\n`;
180+
181+
for (const [eventKind, costsDiff] of Object.entries(summary.events)) {
182+
if (costsDiff.diff_pct !== null) {
183+
const changePercentage = formatPercentage(costsDiff.diff_pct);
184+
const line = `${padRight(eventKind, 20)} ${padLeft(formatNumber(costsDiff.old), 11)}|${padLeft(formatNumber(costsDiff.new), 11)} ${padLeft(changePercentage, 15)}`;
185+
sectionBody += `${line}\n`;
186+
}
187+
}
188+
189+
sectionBody += "```\n</details>\n\n";
190+
191+
if (Math.abs(irDiff.diff_pct) > 5) {
192+
significantChanges = true;
193+
hasSignificantChanges = true;
135194
}
136-
}
137-
138-
commentBody += "```\n</details>\n\n";
139-
140-
if (Math.abs(irDiff.diff_pct) > 5) {
141-
significantChanges = true;
142195
}
143196
}
144197
}
198+
199+
if (hasResults) {
200+
// Wrap section in collapsible details, open only if there are significant changes
201+
const openAttribute = hasSignificantChanges ? " open" : "";
202+
const ruler = isLast ? "" : "\n\n---";
203+
return `<details${openAttribute}>\n<summary><h2>${sectionTitle}</h2></summary>\n\n${sectionBody}${ruler}\n</details>`;
204+
}
205+
return "";
145206
}
146207
147-
const output = `
148-
<details open>
149-
150-
<summary>Performance Benchmark Results</summary>
151-
152-
${commentBody}
153-
154-
</details>
155-
`;
156-
157-
if (significantChanges) {
158-
github.rest.issues.createComment({
159-
issue_number: context.issue.number,
160-
owner: context.repo.owner,
161-
repo: context.repo.repo,
162-
body: output
163-
});
208+
// Process each benchmark category
209+
const sections = [
210+
{ output: compileOutput, title: "🔧 Graph Compilation" },
211+
{ output: updateOutput, title: "🔄 Executor Update" },
212+
{ output: runOnceOutput, title: "🚀 Render: Cold Execution" },
213+
{ output: runCachedOutput, title: "⚡ Render: Cached Execution" }
214+
];
215+
216+
// Generate sections and determine which ones have results
217+
const generatedSections = sections.map(({ output, title }) =>
218+
processBenchmarkOutput(output, title, true) // temporarily mark all as last
219+
).filter(section => section.length > 0);
220+
221+
// Re-generate with correct isLast flags
222+
let sectionIndex = 0;
223+
const finalSections = sections.map(({ output, title }) => {
224+
const section = processBenchmarkOutput(output, title, true); // check if it has results
225+
if (section.length > 0) {
226+
const isLast = sectionIndex === generatedSections.length - 1;
227+
sectionIndex++;
228+
return processBenchmarkOutput(output, title, isLast);
229+
}
230+
return "";
231+
}).filter(section => section.length > 0);
232+
233+
// Combine all sections
234+
commentBody = finalSections.join("\n\n");
235+
236+
if (commentBody.length > 0) {
237+
const output = `<details open>\n<summary>Performance Benchmark Results</summary>\n\n${commentBody}\n</details>`;
238+
239+
if (significantChanges) {
240+
github.rest.issues.createComment({
241+
issue_number: context.issue.number,
242+
owner: context.repo.owner,
243+
repo: context.repo.repo,
244+
body: output
245+
});
246+
} else {
247+
console.log("No significant performance changes detected. Skipping comment.");
248+
console.log(output);
249+
}
164250
} else {
165-
console.log("No significant performance changes detected. Skipping comment.");
166-
console.log(output);
251+
console.log("No benchmark results to display.");
167252
}

node-graph/gstd/src/wasm_application_io.rs

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@ use graphene_svg_renderer::{Render, RenderParams, RenderSvgSegmentList, SvgRende
1616

1717
#[cfg(target_family = "wasm")]
1818
use base64::Engine;
19-
use glam::DAffine2;
2019
use std::sync::Arc;
2120
#[cfg(target_family = "wasm")]
2221
use wasm_bindgen::JsCast;
@@ -196,7 +195,7 @@ async fn render_canvas(render_config: RenderConfig, data: impl Render, editor: &
196195
let frame = SurfaceFrame {
197196
surface_id: surface_handle.window_id,
198197
resolution: render_config.viewport.resolution,
199-
transform: DAffine2::IDENTITY,
198+
transform: glam::DAffine2::IDENTITY,
200199
};
201200

202201
RenderOutputType::CanvasFrame(frame)
@@ -244,7 +243,7 @@ where
244243
};
245244

246245
for row in data.iter_mut() {
247-
*row.transform = DAffine2::from_translation(-aabb.start) * *row.transform;
246+
*row.transform = glam::DAffine2::from_translation(-aabb.start) * *row.transform;
248247
}
249248
data.render_svg(&mut render, &render_params);
250249
render.format_svg(glam::DVec2::ZERO, size);

node-graph/interpreted-executor/benches/benchmark_util.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,10 +3,14 @@ use criterion::measurement::Measurement;
33
use futures::executor::block_on;
44
use graph_craft::proto::ProtoNetwork;
55
use graph_craft::util::{DEMO_ART, compile, load_from_name};
6+
use graphene_std::application_io::EditorApi;
67
use interpreted_executor::dynamic_executor::DynamicExecutor;
8+
use interpreted_executor::util::wrap_network_in_scope;
79

810
pub fn setup_network(name: &str) -> (DynamicExecutor, ProtoNetwork) {
911
let network = load_from_name(name);
12+
let editor_api = std::sync::Arc::new(EditorApi::default());
13+
let network = wrap_network_in_scope(network, editor_api);
1014
let proto_network = compile(network);
1115
let executor = block_on(DynamicExecutor::new(proto_network.clone())).unwrap();
1216
(executor, proto_network)

node-graph/interpreted-executor/benches/run_cached.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,15 +2,15 @@ mod benchmark_util;
22

33
use benchmark_util::{bench_for_each_demo, setup_network};
44
use criterion::{Criterion, criterion_group, criterion_main};
5-
use graphene_std::Context;
5+
use graphene_std::application_io::RenderConfig;
66

77
fn subsequent_evaluations(c: &mut Criterion) {
88
let mut group = c.benchmark_group("Subsequent Evaluations");
9-
let context: Context = None;
9+
let context = RenderConfig::default();
1010
bench_for_each_demo(&mut group, |name, g| {
1111
let (executor, _) = setup_network(name);
1212
g.bench_function(name, |b| {
13-
b.iter(|| futures::executor::block_on(executor.tree().eval_tagged_value(executor.output(), criterion::black_box(context.clone()))).unwrap())
13+
b.iter(|| futures::executor::block_on(executor.tree().eval_tagged_value(executor.output(), criterion::black_box(context))).unwrap())
1414
});
1515
});
1616
group.finish();

node-graph/interpreted-executor/benches/run_cached_iai.rs

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,24 +1,24 @@
1-
use graph_craft::util::*;
2-
use graphene_std::Context;
1+
mod benchmark_util;
2+
3+
use benchmark_util::setup_network;
4+
use graphene_std::application_io::RenderConfig;
35
use iai_callgrind::{black_box, library_benchmark, library_benchmark_group, main};
46
use interpreted_executor::dynamic_executor::DynamicExecutor;
57

68
fn setup_run_cached(name: &str) -> DynamicExecutor {
7-
let network = load_from_name(name);
8-
let proto_network = compile(network);
9-
let executor = futures::executor::block_on(DynamicExecutor::new(proto_network)).unwrap();
9+
let (executor, _) = setup_network(name);
1010

1111
// Warm up the cache by running once
12-
let context: Context = None;
13-
let _ = futures::executor::block_on(executor.tree().eval_tagged_value(executor.output(), context.clone()));
12+
let context = RenderConfig::default();
13+
let _ = futures::executor::block_on(executor.tree().eval_tagged_value(executor.output(), context));
1414

1515
executor
1616
}
1717

1818
#[library_benchmark]
1919
#[benches::with_setup(args = ["isometric-fountain", "painted-dreams", "procedural-string-lights", "parametric-dunescape", "red-dress", "valley-of-spires"], setup = setup_run_cached)]
2020
pub fn run_cached(executor: DynamicExecutor) {
21-
let context: Context = None;
21+
let context = RenderConfig::default();
2222
black_box(futures::executor::block_on(executor.tree().eval_tagged_value(executor.output(), black_box(context))).unwrap());
2323
}
2424

node-graph/interpreted-executor/benches/run_once.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,16 +2,16 @@ mod benchmark_util;
22

33
use benchmark_util::{bench_for_each_demo, setup_network};
44
use criterion::{Criterion, criterion_group, criterion_main};
5-
use graphene_std::Context;
5+
use graphene_std::application_io::RenderConfig;
66

77
fn run_once(c: &mut Criterion) {
88
let mut group = c.benchmark_group("Run Once");
9-
let context: Context = None;
9+
let context = RenderConfig::default();
1010
bench_for_each_demo(&mut group, |name, g| {
1111
g.bench_function(name, |b| {
1212
b.iter_batched(
1313
|| setup_network(name),
14-
|(executor, _)| futures::executor::block_on(executor.tree().eval_tagged_value(executor.output(), criterion::black_box(context.clone()))).unwrap(),
14+
|(executor, _)| futures::executor::block_on(executor.tree().eval_tagged_value(executor.output(), criterion::black_box(context))).unwrap(),
1515
criterion::BatchSize::SmallInput,
1616
)
1717
});

0 commit comments

Comments
 (0)