Skip to content

Commit 43c3ecb

Browse files
detlydetly
committed
Use matrix strategy to measure size data on multiple platforms.
This involves writing the size measurements to separate JSON files (one per matrix job) and retaining them as artifacts, as there's no built-in way to collect all matrix job outputs into a subsequent job's inputs.
1 parent 17fbabb commit 43c3ecb

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)