Skip to content

Commit 2e5cc5f

Browse files
detlydetly
committed
Use matrix strategy to measure size data on multiple platforms.
1 parent 205adce commit 2e5cc5f

File tree

2 files changed

+170
-60
lines changed

2 files changed

+170
-60
lines changed

‎.github/actions/report-code-size-changes/action.yml

Lines changed: 112 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,112 @@
1+
# Github composite action to report on code size changes across different
2+
# platforms.
3+
4+
name: Report binary size changes on PR
5+
description: |
6+
Report on code size changes across different platforms resulting from a PR.
7+
The only input argument is the path to a directory containing a set of
8+
"*.json" files (extension required), each file containing the keys:
9+
10+
- platform: the platform that the code size change was measured on
11+
- reference: the size in bytes of the reference binary (base of PR)
12+
- updated: the size in bytes of the updated binary (head of PR)
13+
14+
The size is reported as a comment on the PR (accessed via context).
15+
inputs:
16+
data-directory:
17+
description: >
18+
Path to directory containing size data as a set of "*.json" files.
19+
required: true
20+
runs:
21+
using: composite
22+
steps:
23+
- name: Post a PR comment if the size has changed
24+
uses: actions/github-script@v6
25+
env:
26+
DATA_DIRECTORY: ${{ inputs.data-directory }}
27+
with:
28+
script: |
29+
const fs = require("fs");
30+
31+
const size_dir = process.env.DATA_DIRECTORY;
32+
33+
// Map the set of all the *.json files into an array of objects.
34+
const globber = await glob.create(`${size_dir}/*.json`);
35+
const files = await globber.glob();
36+
const sizes = files.map(path => {
37+
const contents = fs.readFileSync(path);
38+
return JSON.parse(contents);
39+
});
40+
41+
// Map each object into some text, but only if it shows any difference
42+
// to report.
43+
const size_reports = sizes.flatMap(size_data => {
44+
const platform = size_data["platform"];
45+
const reference = size_data["reference"];
46+
const updated = size_data["updated"];
47+
48+
if (!(reference > 0)) {
49+
core.setFailed(`Reference size invalid: ${reference}`);
50+
return;
51+
}
52+
53+
if (!(updated > 0)) {
54+
core.setFailed(`Updated size invalid: ${updated}`);
55+
return;
56+
}
57+
58+
const formatter = Intl.NumberFormat("en", {
59+
useGrouping: "always"
60+
});
61+
62+
const updated_str = formatter.format(updated);
63+
const reference_str = formatter.format(reference);
64+
65+
const diff = updated - reference;
66+
const diff_pct = (updated / reference) - 1;
67+
68+
const diff_str = Intl.NumberFormat("en", {
69+
useGrouping: "always",
70+
sign: "exceptZero"
71+
}).format(diff);
72+
73+
const diff_pct_str = Intl.NumberFormat("en", {
74+
style: "percent",
75+
useGrouping: "always",
76+
sign: "exceptZero",
77+
maximumFractionDigits: 2
78+
}).format(diff_pct);
79+
80+
if (diff !== 0) {
81+
// The body is created here and wrapped so "weirdly" to avoid whitespace at the start of the lines,
82+
// which is interpreted as a code block by Markdown.
83+
const report = `On platform \`${platform}\`:
84+
85+
- Original binary size: **${reference_str} B**
86+
- Updated binary size: **${updated_str} B**
87+
- Difference: **${diff_str} B** (${diff_pct_str})
88+
89+
`;
90+
91+
return [report];
92+
} else {
93+
return [];
94+
}
95+
});
96+
97+
// If there are any size changes to report, format a comment and post
98+
// it.
99+
if (size_reports.length > 0) {
100+
const comment_sizes = size_reports.join("");
101+
const body = `Code size changes for a hello-world Rust program linked with libstd with backtrace:
102+
103+
${comment_sizes}`;
104+
105+
github.rest.issues.createComment({
106+
issue_number: context.issue.number,
107+
owner: context.repo.owner,
108+
repo: context.repo.repo,
109+
body
110+
});
111+
}
112+

‎.github/workflows/check-binary-size.yml

Lines changed: 58 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -9,13 +9,20 @@ on:
99
branches:
1010
- master
1111

12+
# Both the "measure" and "report" jobs need to know this.
13+
env:
14+
SIZE_DATA_DIR: sizes
15+
1216
# Responsibility is divided between two jobs "measure" and "report", so that the
1317
# job that builds (and potentnially runs) untrusted code does not have PR write
1418
# permission, and vice-versa.
1519
jobs:
1620
measure:
1721
name: Check binary size
18-
runs-on: ubuntu-latest
22+
strategy:
23+
matrix:
24+
platform: [ubuntu-latest, windows-latest]
25+
runs-on: ${{ matrix.platform }}
1926
permissions:
2027
contents: read
2128
env:
@@ -26,9 +33,7 @@ jobs:
2633
TEST_MAIN_RS: foo.rs
2734
BASE_COMMIT: ${{ github.event.pull_request.base.sha }}
2835
HEAD_COMMIT: ${{ github.event.pull_request.head.sha }}
29-
outputs:
30-
binary-size-reference: ${{ steps.size-reference.outputs.test-binary-size }}
31-
binary-size-updated: ${{ steps.size-updated.outputs.test-binary-size }}
36+
SIZE_DATA_FILE: size-${{ strategy.job-index }}.json
3237
steps:
3338
- name: Print info
3439
shell: bash
@@ -37,7 +42,7 @@ jobs:
3742
echo "Base SHA: $BASE_COMMIT"
3843
# Note: the backtrace source that's cloned here is NOT the version to be
3944
# patched in to std. It's cloned here to access the Github action for
40-
# building the test binary and measuring its size.
45+
# building and measuring the test binary.
4146
- name: Clone backtrace to access Github action
4247
uses: actions/checkout@v3
4348
with:
@@ -87,66 +92,59 @@ jobs:
8792
main-rs: ${{ env.TEST_MAIN_RS }}
8893
rustc-dir: ${{ env.RUSTC_DIR }}
8994
id: size-updated
95+
# There is no built-in way to "collect" all the outputs of a set of jobs
96+
# run with a matrix strategy. Subsequent jobs that have a "needs"
97+
# dependency on this one will be run once, when the last matrix job is
98+
# run. Appending data to a single file within a matrix is subject to race
99+
# conditions. So we write the size data to files with distinct names
100+
# generated from the job index.
101+
- name: Write sizes to file
102+
uses: actions/github-script@v6
103+
env:
104+
SIZE_REFERENCE: ${{ steps.size-reference.outputs.test-binary-size }}
105+
SIZE_UPDATED: ${{ steps.size-updated.outputs.test-binary-size }}
106+
PLATFORM: ${{ matrix.platform }}
107+
with:
108+
script: |
109+
const fs = require("fs");
110+
const path = require("path");
111+
112+
fs.mkdirSync(process.env.SIZE_DATA_DIR, {recursive: true});
113+
114+
const output_data = JSON.stringify({
115+
platform: process.env.PLATFORM,
116+
reference: process.env.SIZE_REFERENCE,
117+
updated: process.env.SIZE_UPDATED,
118+
});
119+
120+
// The "wx" flag makes this fail if the file exists, which we want,
121+
// because there should be no collisions.
122+
fs.writeFileSync(
123+
path.join(process.env.SIZE_DATA_DIR, process.env.SIZE_DATA_FILE),
124+
output_data,
125+
{ flag: "wx" },
126+
);
127+
- name: Upload size data
128+
uses: actions/upload-artifact@v3
129+
with:
130+
name: size-files
131+
path: ${{ env.SIZE_DATA_DIR }}/${{ env.SIZE_DATA_FILE }}
132+
retention-days: 1
133+
if-no-files-found: error
90134
report:
91135
name: Report binary size changes
92136
runs-on: ubuntu-latest
93137
needs: measure
94138
permissions:
95139
pull-requests: write
96140
steps:
97-
- name: Post a PR comment if the size has changed
98-
uses: actions/github-script@v6
99-
env:
100-
SIZE_REFERENCE: ${{ needs.measure.outputs.binary-size-reference }}
101-
SIZE_UPDATED: ${{ needs.measure.outputs.binary-size-updated }}
141+
# Clone backtrace to access Github composite actions to report size.
142+
- uses: actions/checkout@v3
143+
- name: Download size data
144+
uses: actions/download-artifact@v3
102145
with:
103-
script: |
104-
const reference = process.env.SIZE_REFERENCE;
105-
const updated = process.env.SIZE_UPDATED;
106-
107-
if (!(reference > 0)) {
108-
core.setFailed(`Reference size invalid: ${reference}`);
109-
return;
110-
}
111-
112-
if (!(updated > 0)) {
113-
core.setFailed(`Updated size invalid: ${updated}`);
114-
return;
115-
}
116-
117-
const formatter = Intl.NumberFormat("en", {useGrouping: "always"});
118-
119-
const updated_str = formatter.format(updated);
120-
const reference_str = formatter.format(reference);
121-
122-
const diff = updated - reference;
123-
const diff_pct = (updated / reference) - 1;
124-
125-
const diff_str = Intl.NumberFormat("en", {
126-
useGrouping: "always",
127-
sign: "exceptZero"
128-
}).format(diff);
129-
130-
const diff_pct_str = Intl.NumberFormat("en", {
131-
style: "percent",
132-
useGrouping: "always",
133-
sign: "exceptZero",
134-
maximumFractionDigits: 2
135-
}).format(diff_pct);
136-
137-
if (diff !== 0) {
138-
// The body is created here and wrapped so "weirdly" to avoid whitespace at the start of the lines,
139-
// which is interpreted as a code block by Markdown.
140-
const body = `Below is the size of a hello-world Rust program linked with libstd with backtrace.
141-
142-
Original binary size: **${reference_str} B**
143-
Updated binary size: **${updated_str} B**
144-
Difference: **${diff_str} B** (${diff_pct_str})`;
145-
146-
github.rest.issues.createComment({
147-
issue_number: context.issue.number,
148-
owner: context.repo.owner,
149-
repo: context.repo.repo,
150-
body
151-
})
152-
}
146+
name: size-files
147+
path: ${{ env.SIZE_DATA_DIR }}
148+
- uses: ./.github/actions/report-code-size-changes
149+
with:
150+
data-directory: ${{ env.SIZE_DATA_DIR }}

0 commit comments

Comments
 (0)