Skip to content

Commit 7b78a4f

Browse files
workingjubileeworkingjubilee
authored
Modularise CI workflow and validate outputs for binary size checks
From detly/master in #549
2 parents b471e5f + 43c3ecb commit 7b78a4f

File tree

3 files changed

+279
-52
lines changed

3 files changed

+279
-52
lines changed

‎.github/actions/build-with-patched-std/action.yml

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
# Github composite action to build a single-source-file test binary with an
2+
# already-checked-out version of Rust's stdlib, that will be patched with a
3+
# given revision of the backtrace crate.
4+
5+
name: Build with patched std
6+
description: >
7+
Build a binary with a version of std that's had a specific revision of
8+
backtrace patched in.
9+
inputs:
10+
backtrace-commit:
11+
description: The git commit of backtrace to patch in to std
12+
required: true
13+
main-rs:
14+
description: The (single) source code file to compile
15+
required: true
16+
rustc-dir:
17+
description: The root directory of the rustc repo
18+
required: true
19+
outputs:
20+
test-binary-size:
21+
description: The size in bytes of the built test binary
22+
value: ${{ steps.measure.outputs.test-binary-size }}
23+
runs:
24+
using: composite
25+
steps:
26+
- shell: bash
27+
id: measure
28+
env:
29+
RUSTC_FLAGS: -Copt-level=3 -Cstrip=symbols
30+
# This symlink is made by Build::new() in the bootstrap crate, using a
31+
# symlink on Linux and a junction on Windows, so it will exist on both
32+
# platforms.
33+
RUSTC_BUILD_DIR: build/host
34+
working-directory: ${{ inputs.rustc-dir }}
35+
run: |
36+
rm -rf "$RUSTC_BUILD_DIR/stage0-std"
37+
38+
(cd library/backtrace && git checkout ${{ inputs.backtrace-commit }})
39+
git add library/backtrace
40+
41+
python3 x.py build library --stage 0
42+
43+
TEMP_BUILD_OUTPUT=$(mktemp test-binary-XXXXXXXX)
44+
"$RUSTC_BUILD_DIR/stage0-sysroot/bin/rustc" $RUSTC_FLAGS "${{ inputs.main-rs }}" -o "$TEMP_BUILD_OUTPUT"
45+
BINARY_SIZE=$(stat -c '%s' "$TEMP_BUILD_OUTPUT")
46+
rm "$TEMP_BUILD_OUTPUT"
47+
48+
echo "test-binary-size=$BINARY_SIZE" >> "$GITHUB_OUTPUT"

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

Lines changed: 111 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,111 @@
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+
}

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

Lines changed: 120 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -9,75 +9,143 @@ 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+
16+
# Responsibility is divided between two jobs "measure" and "report", so that the
17+
# job that builds (and potentnially runs) untrusted code does not have PR write
18+
# permission, and vice-versa.
1219
jobs:
13-
test:
20+
measure:
1421
name: Check binary size
15-
runs-on: ubuntu-latest
22+
strategy:
23+
matrix:
24+
platform: [ubuntu-latest, windows-latest]
25+
runs-on: ${{ matrix.platform }}
1626
permissions:
17-
pull-requests: write
27+
contents: read
28+
env:
29+
# This cannot be used as a context variable in the 'uses' key later. If it
30+
# changes, update those steps too.
31+
BACKTRACE_DIR: backtrace
32+
RUSTC_DIR: rustc
33+
TEST_MAIN_RS: foo.rs
34+
BASE_COMMIT: ${{ github.event.pull_request.base.sha }}
35+
HEAD_COMMIT: ${{ github.event.pull_request.head.sha }}
36+
SIZE_DATA_FILE: size-${{ strategy.job-index }}.json
1837
steps:
1938
- name: Print info
39+
shell: bash
2040
run: |
21-
echo "Current SHA: ${{ github.event.pull_request.head.sha }}"
22-
echo "Base SHA: ${{ github.event.pull_request.base.sha }}"
41+
echo "Current SHA: $HEAD_COMMIT"
42+
echo "Base SHA: $BASE_COMMIT"
43+
# Note: the backtrace source that's cloned here is NOT the version to be
44+
# patched in to std. It's cloned here to access the Github action for
45+
# building and measuring the test binary.
46+
- name: Clone backtrace to access Github action
47+
uses: actions/checkout@v3
48+
with:
49+
path: ${{ env.BACKTRACE_DIR }}
2350
- name: Clone Rustc
2451
uses: actions/checkout@v3
2552
with:
2653
repository: rust-lang/rust
27-
fetch-depth: 1
28-
- name: Fetch backtrace
29-
run: git submodule update --init library/backtrace
30-
- name: Create hello world program that uses backtrace
31-
run: printf "fn main() { panic!(); }" > foo.rs
32-
- name: Build binary with base version of backtrace
54+
path: ${{ env.RUSTC_DIR }}
55+
- name: Set up std repository and backtrace submodule for size test
56+
shell: bash
57+
working-directory: ${{ env.RUSTC_DIR }}
58+
env:
59+
PR_SOURCE_REPO: ${{ github.event.pull_request.head.repo.full_name }}
3360
run: |
34-
printf "[llvm]\ndownload-ci-llvm = true\n\n[rust]\nincremental = false\n" > config.toml
61+
# Bootstrap config
62+
cat <<EOF > config.toml
63+
[llvm]
64+
download-ci-llvm = true
65+
[rust]
66+
incremental = false
67+
EOF
68+
69+
# Test program source
70+
cat <<EOF > $TEST_MAIN_RS
71+
fn main() {
72+
panic!();
73+
}
74+
EOF
75+
76+
git submodule update --init library/backtrace
77+
3578
cd library/backtrace
36-
git remote add head-pr https://github.com/${{ github.event.pull_request.head.repo.full_name }}
79+
git remote add head-pr "https://github.com/$PR_SOURCE_REPO"
3780
git fetch --all
38-
git checkout ${{ github.event.pull_request.base.sha }}
39-
cd ../..
40-
git add library/backtrace
41-
python3 x.py build library --stage 0
42-
./build/x86_64-unknown-linux-gnu/stage0-sysroot/bin/rustc -O foo.rs -o binary-reference
81+
- name: Build binary with base version of backtrace
82+
uses: ./backtrace/.github/actions/build-with-patched-std
83+
with:
84+
backtrace-commit: ${{ env.BASE_COMMIT }}
85+
main-rs: ${{ env.TEST_MAIN_RS }}
86+
rustc-dir: ${{ env.RUSTC_DIR }}
87+
id: size-reference
4388
- name: Build binary with PR version of backtrace
44-
run: |
45-
cd library/backtrace
46-
git checkout ${{ github.event.pull_request.head.sha }}
47-
cd ../..
48-
git add library/backtrace
49-
rm -rf build/x86_64-unknown-linux-gnu/stage0-std
50-
python3 x.py build library --stage 0
51-
./build/x86_64-unknown-linux-gnu/stage0-sysroot/bin/rustc -O foo.rs -o binary-updated
52-
- name: Display binary size
53-
run: |
54-
ls -la binary-*
55-
echo "SIZE_REFERENCE=$(stat -c '%s' binary-reference)" >> "$GITHUB_ENV"
56-
echo "SIZE_UPDATED=$(stat -c '%s' binary-updated)" >> "$GITHUB_ENV"
57-
- name: Post a PR comment if the size has changed
89+
uses: ./backtrace/.github/actions/build-with-patched-std
90+
with:
91+
backtrace-commit: ${{ env.HEAD_COMMIT }}
92+
main-rs: ${{ env.TEST_MAIN_RS }}
93+
rustc-dir: ${{ env.RUSTC_DIR }}
94+
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
58102
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 }}
59107
with:
60108
script: |
61-
const reference = process.env.SIZE_REFERENCE;
62-
const updated = process.env.SIZE_UPDATED;
63-
const diff = updated - reference;
64-
const plus = diff > 0 ? "+" : "";
65-
const diff_str = `${plus}${diff}B`;
109+
const fs = require("fs");
110+
const path = require("path");
66111
67-
if (diff !== 0) {
68-
const percent = (((updated / reference) - 1) * 100).toFixed(2);
69-
// The body is created here and wrapped so "weirdly" to avoid whitespace at the start of the lines,
70-
// which is interpreted as a code block by Markdown.
71-
const body = `Below is the size of a hello-world Rust program linked with libstd with backtrace.
112+
fs.mkdirSync(process.env.SIZE_DATA_DIR, {recursive: true});
72113
73-
Original binary size: **${reference}B**
74-
Updated binary size: **${updated}B**
75-
Difference: **${diff_str}** (${percent}%)`;
114+
const output_data = JSON.stringify({
115+
platform: process.env.PLATFORM,
116+
reference: process.env.SIZE_REFERENCE,
117+
updated: process.env.SIZE_UPDATED,
118+
});
76119
77-
github.rest.issues.createComment({
78-
issue_number: context.issue.number,
79-
owner: context.repo.owner,
80-
repo: context.repo.repo,
81-
body
82-
})
83-
}
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
134+
report:
135+
name: Report binary size changes
136+
runs-on: ubuntu-latest
137+
needs: measure
138+
permissions:
139+
pull-requests: write
140+
steps:
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
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
150+
with:
151+
data-directory: ${{ env.SIZE_DATA_DIR }}

0 commit comments

Comments
 (0)