Skip to content

Commit 6703145

Browse files
detlydetly
committed
Use matrix strategy to measure size data on multiple platforms.
1 parent 17fbabb commit 6703145

File tree

2 files changed

+140
-52
lines changed

2 files changed

+140
-52
lines changed

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

Lines changed: 83 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -1,71 +1,111 @@
1-
# Github composite action to report on code size changes
1+
# Github composite action to report on code size changes across different
2+
# platforms.
3+
24
name: Report binary size changes on PR
35
description: |
4-
Report on code size changes resulting from a PR as a comment on the PR
5-
(accessed via context).
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).
615
inputs:
7-
reference:
8-
description: The size in bytes of the reference binary (base of PR).
9-
required: true
10-
updated:
11-
description: The size in bytes of the updated binary (head of PR).
16+
data-directory:
17+
description: >
18+
Path to directory containing size data as a set of "*.json" files.
1219
required: true
1320
runs:
1421
using: composite
1522
steps:
1623
- name: Post a PR comment if the size has changed
1724
uses: actions/github-script@v6
1825
env:
19-
SIZE_REFERENCE: ${{ inputs.reference }}
20-
SIZE_UPDATED: ${{ inputs.updated }}
26+
DATA_DIRECTORY: ${{ inputs.data-directory }}
2127
with:
2228
script: |
23-
const reference = process.env.SIZE_REFERENCE;
24-
const updated = process.env.SIZE_UPDATED;
29+
const fs = require("fs");
2530
26-
if (!(reference > 0)) {
27-
core.setFailed(`Reference size invalid: ${reference}`);
28-
return;
29-
}
31+
const size_dir = process.env.DATA_DIRECTORY;
3032
31-
if (!(updated > 0)) {
32-
core.setFailed(`Updated size invalid: ${updated}`);
33-
return;
34-
}
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);
3572
36-
const formatter = Intl.NumberFormat("en", {useGrouping: "always"});
73+
const diff_pct_str = Intl.NumberFormat("en", {
74+
style: "percent",
75+
useGrouping: "always",
76+
sign: "exceptZero",
77+
maximumFractionDigits: 2
78+
}).format(diff_pct);
3779
38-
const updated_str = formatter.format(updated);
39-
const reference_str = formatter.format(reference);
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}\`:
4084
41-
const diff = updated - reference;
42-
const diff_pct = (updated / reference) - 1;
85+
- Original binary size: **${reference_str} B**
86+
- Updated binary size: **${updated_str} B**
87+
- Difference: **${diff_str} B** (${diff_pct_str})
4388
44-
const diff_str = Intl.NumberFormat("en", {
45-
useGrouping: "always",
46-
sign: "exceptZero"
47-
}).format(diff);
89+
`;
4890
49-
const diff_pct_str = Intl.NumberFormat("en", {
50-
style: "percent",
51-
useGrouping: "always",
52-
sign: "exceptZero",
53-
maximumFractionDigits: 2
54-
}).format(diff_pct);
91+
return [report];
92+
} else {
93+
return [];
94+
}
95+
});
5596
56-
if (diff !== 0) {
57-
// The body is created here and wrapped so "weirdly" to avoid whitespace at the start of the lines,
58-
// which is interpreted as a code block by Markdown.
59-
const body = `Below is the size of a hello-world Rust program linked with libstd with backtrace.
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:
60102
61-
Original binary size: **${reference_str} B**
62-
Updated binary size: **${updated_str} B**
63-
Difference: **${diff_str} B** (${diff_pct_str})`;
103+
${comment_sizes}`;
64104
65105
github.rest.issues.createComment({
66106
issue_number: context.issue.number,
67107
owner: context.repo.owner,
68108
repo: context.repo.repo,
69109
body
70-
})
110+
});
71111
}

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

Lines changed: 57 additions & 9 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,6 +92,45 @@ 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
@@ -96,8 +140,12 @@ jobs:
96140
steps:
97141
# Clone backtrace to access Github composite actions to report size.
98142
- uses: actions/checkout@v3
99-
# Run the size reporting action.
100-
- uses: ./.github/actions/report-code-size-changes
143+
- name: Download size data
144+
uses: actions/download-artifact@v3
145+
with:
146+
name: size-files
147+
path: ${{ env.SIZE_DATA_DIR }}
148+
- name: Analyze and report size changes
149+
uses: ./.github/actions/report-code-size-changes
101150
with:
102-
reference: ${{ needs.measure.outputs.binary-size-reference }}
103-
updated: ${{ needs.measure.outputs.binary-size-updated }}
151+
data-directory: ${{ env.SIZE_DATA_DIR }}

0 commit comments

Comments
 (0)