Skip to content

Commit f1f03cc

Browse files
authored
fix: improve CI benchmark table in PR (#1663)
1 parent 9324df7 commit f1f03cc

File tree

5 files changed

+401
-44
lines changed

5 files changed

+401
-44
lines changed
Lines changed: 248 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,248 @@
1+
import pathlib, re, sys
2+
3+
try:
4+
p = pathlib.Path("comparison.md")
5+
if not p.exists():
6+
print("comparison.md not found, skipping post-processing.")
7+
sys.exit(0)
8+
9+
lines = p.read_text(encoding="utf-8").splitlines()
10+
processed_lines = []
11+
in_code = False
12+
def strip_worker_suffix(text: str) -> str:
13+
return re.sub(r'(\S+?)-\d+(\s|$)', r'\1\2', text)
14+
15+
def get_icon(diff_val: float) -> str:
16+
if diff_val > 10:
17+
return "🐌"
18+
if diff_val < -10:
19+
return "🚀"
20+
return "➡️"
21+
22+
def clean_superscripts(text: str) -> str:
23+
return re.sub(r'[¹²³⁴⁵⁶⁷⁸⁹⁰]', '', text)
24+
25+
def parse_val(token: str):
26+
if '%' in token or '=' in token:
27+
return None
28+
token = clean_superscripts(token)
29+
token = token.split('±')[0].strip()
30+
token = token.split('(')[0].strip()
31+
if not token:
32+
return None
33+
34+
m = re.match(r'^([-+]?\d*\.?\d+)([a-zA-Zµ]+)?$', token)
35+
if not m:
36+
return None
37+
try:
38+
val = float(m.group(1))
39+
except ValueError:
40+
return None
41+
suffix = (m.group(2) or "").replace("µ", "u")
42+
multipliers = {
43+
"n": 1e-9,
44+
"ns": 1e-9,
45+
"u": 1e-6,
46+
"us": 1e-6,
47+
"m": 1e-3,
48+
"ms": 1e-3,
49+
"s": 1.0,
50+
"k": 1e3,
51+
"K": 1e3,
52+
"M": 1e6,
53+
"G": 1e9,
54+
"Ki": 1024.0,
55+
"Mi": 1024.0**2,
56+
"Gi": 1024.0**3,
57+
"Ti": 1024.0**4,
58+
"B": 1.0,
59+
"B/op": 1.0,
60+
"C": 1.0, # tolerate degree/unit markers that don't affect ratio
61+
}
62+
return val * multipliers.get(suffix, 1.0)
63+
64+
def extract_two_numbers(tokens):
65+
found = []
66+
for t in tokens[1:]: # skip name
67+
if t in {"±", "∞", "~", "│", "│"}:
68+
continue
69+
if '%' in t or '=' in t:
70+
continue
71+
val = parse_val(t)
72+
if val is not None:
73+
found.append(val)
74+
if len(found) == 2:
75+
break
76+
return found
77+
78+
# Pass 0:
79+
# 1. find a header line with pipes to derive alignment hint
80+
# 2. calculate max content width to ensure right-most alignment
81+
max_content_width = 0
82+
83+
for line in lines:
84+
if line.strip() == "```":
85+
in_code = not in_code
86+
continue
87+
if not in_code:
88+
continue
89+
90+
# Skip footnotes/meta for width calculation
91+
if re.match(r'^\s*[¹²³⁴⁵⁶⁷⁸⁹⁰]', line) or re.search(r'need\s*>?=\s*\d+\s+samples', line):
92+
continue
93+
if not line.strip() or line.strip().startswith(('goos:', 'goarch:', 'pkg:', 'cpu:')):
94+
continue
95+
# Header lines are handled separately in Pass 1
96+
if '│' in line and ('vs base' in line or 'old' in line or 'new' in line):
97+
continue
98+
99+
# It's likely a data line
100+
# Check if it has an existing percentage we might move/align
101+
curr_line = strip_worker_suffix(line).rstrip()
102+
pct_match = re.search(r'([+-]?\d+\.\d+)%', curr_line)
103+
if pct_match:
104+
# If we are going to realign this, we count width up to the percentage
105+
w = len(curr_line[:pct_match.start()].rstrip())
106+
else:
107+
w = len(curr_line)
108+
109+
if w > max_content_width:
110+
max_content_width = w
111+
112+
# Calculate global alignment target for Diff column
113+
# Ensure target column is beyond the longest line with some padding
114+
diff_col_start = max_content_width + 4
115+
116+
# Calculate right boundary (pipe) position
117+
# Diff column width ~12 chars (e.g. "+100.00% 🚀")
118+
right_boundary = diff_col_start + 14
119+
120+
# Reset code fence tracking state for Pass 1
121+
in_code = False
122+
for line in lines:
123+
124+
if line.strip() == "```":
125+
in_code = not in_code
126+
processed_lines.append(line)
127+
continue
128+
129+
if not in_code:
130+
processed_lines.append(line)
131+
continue
132+
133+
# footnotes keep untouched
134+
if re.match(r'^\s*[¹²³⁴⁵⁶⁷⁸⁹⁰]', line) or re.search(r'need\s*>?=\s*\d+\s+samples', line):
135+
processed_lines.append(line)
136+
continue
137+
138+
# header lines: ensure last column labeled Diff and force alignment
139+
if '│' in line and ('vs base' in line or 'old' in line or 'new' in line):
140+
# Strip trailing pipe and whitespace
141+
stripped_header = line.rstrip().rstrip('│').rstrip()
142+
143+
# If "vs base" is present, ensure we don't duplicate "Diff" if it's already there
144+
# But we want to enforce OUR alignment, so we might strip existing Diff
145+
stripped_header = re.sub(r'\s+Diff\s*$', '', stripped_header, flags=re.IGNORECASE)
146+
stripped_header = re.sub(r'\s+Delta\b', '', stripped_header, flags=re.IGNORECASE)
147+
148+
# Pad to diff_col_start
149+
if len(stripped_header) < diff_col_start:
150+
new_header = stripped_header + " " * (diff_col_start - len(stripped_header))
151+
else:
152+
new_header = stripped_header + " "
153+
154+
# Add Diff column header if it's the second header row (vs base)
155+
if 'vs base' in line:
156+
new_header += "Diff"
157+
158+
# Add closing pipe at the right boundary
159+
current_len = len(new_header)
160+
if current_len < right_boundary:
161+
new_header += " " * (right_boundary - current_len)
162+
163+
new_header += "│"
164+
processed_lines.append(new_header)
165+
continue
166+
167+
# non-data meta lines
168+
if not line.strip() or line.strip().startswith(('goos:', 'goarch:', 'pkg:')):
169+
processed_lines.append(line)
170+
continue
171+
172+
line = strip_worker_suffix(line)
173+
tokens = line.split()
174+
if not tokens:
175+
processed_lines.append(line)
176+
continue
177+
178+
numbers = extract_two_numbers(tokens)
179+
pct_match = re.search(r'([+-]?\d+\.\d+)%', line)
180+
181+
# Helper to align and append
182+
def append_aligned(left_part, content):
183+
if len(left_part) < diff_col_start:
184+
aligned = left_part + " " * (diff_col_start - len(left_part))
185+
else:
186+
aligned = left_part + " "
187+
188+
# Ensure content doesn't exceed right boundary (visual check only, we don't truncate)
189+
# But users asked not to exceed header pipe.
190+
# Header pipe is at right_boundary.
191+
# Content starts at diff_col_start.
192+
# So content length should be <= right_boundary - diff_col_start
193+
return f"{aligned}{content}"
194+
195+
# Special handling for geomean when values missing or zero
196+
is_geomean = tokens[0] == "geomean"
197+
if is_geomean and (len(numbers) < 2 or any(v == 0 for v in numbers)) and not pct_match:
198+
leading = re.match(r'^\s*', line).group(0)
199+
left = f"{leading}geomean"
200+
processed_lines.append(append_aligned(left, "n/a (has zero)"))
201+
continue
202+
203+
# when both values are zero, force diff = 0 and align
204+
if len(numbers) == 2 and numbers[0] == 0 and numbers[1] == 0:
205+
diff_val = 0.0
206+
icon = get_icon(diff_val)
207+
left = line.rstrip()
208+
processed_lines.append(append_aligned(left, f"{diff_val:+.2f}% {icon}"))
209+
continue
210+
211+
# recompute diff when we have two numeric values
212+
if len(numbers) == 2 and numbers[0] != 0:
213+
diff_val = (numbers[1] - numbers[0]) / numbers[0] * 100
214+
icon = get_icon(diff_val)
215+
216+
left = line
217+
if pct_match:
218+
left = line[:pct_match.start()].rstrip()
219+
else:
220+
left = line.rstrip()
221+
222+
processed_lines.append(append_aligned(left, f"{diff_val:+.2f}% {icon}"))
223+
continue
224+
225+
# fallback: align existing percentage to Diff column and (re)append icon
226+
if pct_match:
227+
try:
228+
pct_val = float(pct_match.group(1))
229+
icon = get_icon(pct_val)
230+
231+
left = line[:pct_match.start()].rstrip()
232+
suffix = line[pct_match.end():]
233+
# Remove any existing icon after the percentage to avoid duplicates
234+
suffix = re.sub(r'\s*(🐌|🚀|➡️)', '', suffix)
235+
236+
processed_lines.append(append_aligned(left, f"{pct_val:+.2f}% {icon}{suffix}"))
237+
except ValueError:
238+
processed_lines.append(line)
239+
continue
240+
241+
# If we cannot parse numbers or percentages, keep the original (only worker suffix stripped)
242+
processed_lines.append(line)
243+
244+
p.write_text("\n".join(processed_lines) + "\n", encoding="utf-8")
245+
246+
except Exception as e:
247+
print(f"Error post-processing comparison.md: {e}")
248+
sys.exit(1)
Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
module.exports = async ({github, context, core}) => {
2+
try {
3+
const artifacts = await github.rest.actions.listWorkflowRunArtifacts({
4+
owner: context.repo.owner,
5+
repo: context.repo.repo,
6+
run_id: context.payload.workflow_run.id,
7+
});
8+
9+
const matchArtifact = artifacts.data.artifacts.find((artifact) => {
10+
return artifact.name == "benchmark-results";
11+
});
12+
13+
if (!matchArtifact) {
14+
core.setFailed("No artifact named 'benchmark-results' found.");
15+
return;
16+
}
17+
18+
const download = await github.rest.actions.downloadArtifact({
19+
owner: context.repo.owner,
20+
repo: context.repo.repo,
21+
artifact_id: matchArtifact.id,
22+
archive_format: 'zip',
23+
});
24+
25+
const fs = require('fs');
26+
const path = require('path');
27+
const workspace = process.env.GITHUB_WORKSPACE;
28+
fs.writeFileSync(path.join(workspace, 'benchmark-results.zip'), Buffer.from(download.data));
29+
} catch (error) {
30+
core.setFailed(`Failed to download artifact: ${error.message}`);
31+
}
32+
};

.github/scripts/post_comment.js

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
1+
module.exports = async ({github, context, core}) => {
2+
const fs = require('fs');
3+
4+
// Validate pr_number.txt
5+
if (!fs.existsSync('pr_number.txt')) {
6+
core.setFailed("Required artifact file 'pr_number.txt' was not found in the workspace.");
7+
return;
8+
}
9+
const prNumberContent = fs.readFileSync('pr_number.txt', 'utf8').trim();
10+
const issue_number = parseInt(prNumberContent, 10);
11+
if (!Number.isFinite(issue_number) || issue_number <= 0) {
12+
core.setFailed('Invalid PR number in pr_number.txt: "' + prNumberContent + '"');
13+
return;
14+
}
15+
16+
// Validate comparison.md
17+
if (!fs.existsSync('comparison.md')) {
18+
core.setFailed("Required artifact file 'comparison.md' was not found in the workspace.");
19+
return;
20+
}
21+
let comparison;
22+
try {
23+
comparison = fs.readFileSync('comparison.md', 'utf8');
24+
} catch (error) {
25+
core.setFailed("Failed to read 'comparison.md': " + error.message);
26+
return;
27+
}
28+
29+
// Find existing comment
30+
const { data: comments } = await github.rest.issues.listComments({
31+
owner: context.repo.owner,
32+
repo: context.repo.repo,
33+
issue_number: issue_number,
34+
});
35+
36+
const botComment = comments.find(comment =>
37+
comment.user.type === 'Bot' &&
38+
comment.body.includes('Benchmark Comparison')
39+
);
40+
41+
const footer = '<sub>🤖 This comment will be automatically updated with the latest benchmark results.</sub>';
42+
const commentBody = `${comparison}\n\n${footer}`;
43+
44+
if (botComment) {
45+
await github.rest.issues.updateComment({
46+
owner: context.repo.owner,
47+
repo: context.repo.repo,
48+
comment_id: botComment.id,
49+
body: commentBody
50+
});
51+
} else {
52+
await github.rest.issues.createComment({
53+
owner: context.repo.owner,
54+
repo: context.repo.repo,
55+
issue_number: issue_number,
56+
body: commentBody
57+
});
58+
}
59+
};

.github/workflows/comment.yml

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
name: Post Benchmark Comment
2+
3+
on:
4+
workflow_run:
5+
workflows: ["Performance Comparison for Pull Requests"]
6+
types:
7+
- completed
8+
9+
permissions:
10+
pull-requests: write
11+
12+
jobs:
13+
comment:
14+
runs-on: ubuntu-latest
15+
if: >
16+
github.event.workflow_run.event == 'pull_request' &&
17+
github.event.workflow_run.conclusion == 'success'
18+
steps:
19+
- name: Checkout repo
20+
uses: actions/checkout@v4
21+
22+
- name: 'Download artifact'
23+
uses: actions/github-script@v7
24+
with:
25+
script: |
26+
const script = require('./.github/scripts/download_artifact.js')
27+
await script({github, context, core})
28+
29+
- name: 'Unzip artifact'
30+
run: unzip benchmark-results.zip
31+
32+
- name: 'Post comment'
33+
uses: actions/github-script@v7
34+
with:
35+
script: |
36+
const script = require('./.github/scripts/post_comment.js')
37+
await script({github, context, core})

0 commit comments

Comments
 (0)