Skip to content

Commit 1edb897

Browse files
authored
Merge pull request #15 from jserv/lto
Add per-PR subsystem-size budget gate with PR-comment surface
2 parents 42c804d + 19340a7 commit 1edb897

7 files changed

Lines changed: 1011 additions & 138 deletions

File tree

.github/workflows/main.yml

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,9 @@ on:
99
env:
1010
FORCE_JAVASCRIPT_ACTIONS_TO_NODE24: true
1111

12+
permissions:
13+
contents: read
14+
1215
concurrency:
1316
group: build-${{ github.workflow }}-${{ github.ref }}
1417
cancel-in-progress: true
@@ -75,6 +78,26 @@ jobs:
7578
QUIET: 1
7679
run: ./build.sh
7780

81+
- name: Build diagnostic subsystem rollup
82+
if: success()
83+
env:
84+
QUIET: 1
85+
KERNEL_DEBUG_INFO: reduced
86+
run: ./build.sh linux bootwrapper
87+
88+
- name: Render subsystem rollup markdown
89+
if: success()
90+
run: |
91+
python3 scripts/rollup-to-markdown.py \
92+
--rollup profiles/kernel-pgo/none/subsystem-rollup.txt \
93+
--budget-status profiles/kernel-pgo/none/subsystem-budget.txt \
94+
--top 12 \
95+
--output profiles/kernel-pgo/none/subsystem-rollup.md
96+
97+
- name: Add subsystem rollup to step summary
98+
if: success()
99+
run: cat profiles/kernel-pgo/none/subsystem-rollup.md >> "$GITHUB_STEP_SUMMARY"
100+
78101
- name: Save source downloads cache
79102
if: always() && steps.restore-downloads.outputs.cache-hit != 'true'
80103
uses: actions/cache/save@v5
@@ -127,6 +150,21 @@ jobs:
127150
logs
128151
if-no-files-found: ignore
129152

153+
- name: Upload subsystem rollup artifacts
154+
if: success()
155+
uses: actions/upload-artifact@v7
156+
with:
157+
name: subsystem-rollup
158+
compression-level: 0
159+
path: |
160+
profiles/kernel-pgo/none/subsystem-rollup.txt
161+
profiles/kernel-pgo/none/subsystem-rollup.md
162+
profiles/kernel-pgo/none/subsystem-budget.txt
163+
profiles/kernel-pgo/none/subsystem-rollup-tree.html
164+
profiles/kernel-pgo/none/subsystem-rollup-deep.html
165+
profiles/kernel-pgo/none/subsystem-rollup-bars.svg
166+
if-no-files-found: warn
167+
130168
qemu:
131169
name: Validate boot in QEMU
132170
runs-on: ubuntu-24.04
Lines changed: 120 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,120 @@
1+
name: PR comment - subsystem rollup
2+
3+
# Posts the diagnostic subsystem-size rollup as a PR comment after the
4+
# Build workflow finishes. Lives in a separate workflow on purpose:
5+
# `workflow_run` runs in the base-repo trusted context, so this is the
6+
# only place pull-requests:write is granted. The Build job stays
7+
# read-only, which keeps a compromised toolchain or build-script step
8+
# from minting comments on attacker-supplied PRs.
9+
#
10+
# Fork PRs work too: workflow_run.pull_requests is empty for forks, so
11+
# the resolver falls back to a head-SHA search.
12+
13+
on:
14+
workflow_run:
15+
workflows: [Build]
16+
types: [completed]
17+
18+
permissions:
19+
contents: read
20+
pull-requests: write
21+
actions: read
22+
23+
jobs:
24+
comment:
25+
name: Upsert subsystem rollup comment
26+
runs-on: ubuntu-24.04
27+
if: |
28+
github.event.workflow_run.event == 'pull_request'
29+
&& github.event.workflow_run.conclusion == 'success'
30+
31+
steps:
32+
- name: Download subsystem rollup artifact
33+
uses: actions/download-artifact@v5
34+
with:
35+
name: subsystem-rollup
36+
path: rollup
37+
run-id: ${{ github.event.workflow_run.id }}
38+
github-token: ${{ github.token }}
39+
40+
- name: Resolve PR number
41+
id: pr
42+
uses: actions/github-script@v8
43+
with:
44+
script: |
45+
const run = context.payload.workflow_run;
46+
// workflow_run.pull_requests is populated for same-repo
47+
// PRs and empty for forks; fall back to a head-SHA search
48+
// so fork PRs still get the comment.
49+
const direct = run.pull_requests || [];
50+
if (direct.length > 0) {
51+
core.setOutput('number', String(direct[0].number));
52+
return;
53+
}
54+
const { owner, repo } = context.repo;
55+
const { data } = await github.rest.search.issuesAndPullRequests({
56+
q: `repo:${owner}/${repo} is:pr is:open ${run.head_sha}`,
57+
});
58+
if (data.items.length === 0) {
59+
core.warning(`no open PR matched head SHA ${run.head_sha}`);
60+
core.setOutput('number', '');
61+
return;
62+
}
63+
core.setOutput('number', String(data.items[0].number));
64+
65+
- name: Upsert comment
66+
if: steps.pr.outputs.number != ''
67+
uses: actions/github-script@v8
68+
env:
69+
# Pass the PR number through the environment instead of
70+
# interpolating it directly into the script body. Template
71+
# substitution happens before the JS engine sees the source,
72+
# so any future change that lets a non-numeric value reach
73+
# this output would otherwise be a script-injection vector.
74+
PR_NUMBER: ${{ steps.pr.outputs.number }}
75+
with:
76+
script: |
77+
const fs = require('fs');
78+
// upload-artifact@v4+ preserves workspace-relative paths,
79+
// so the markdown lives at its original profiles/ path
80+
// inside the downloaded folder.
81+
const body = fs.readFileSync(
82+
'rollup/profiles/kernel-pgo/none/subsystem-rollup.md',
83+
'utf8',
84+
);
85+
const marker = '<!-- subsystem-rollup-comment -->';
86+
const { owner, repo } = context.repo;
87+
const issue_number = Number(process.env.PR_NUMBER);
88+
const comments = await github.paginate(
89+
github.rest.issues.listComments,
90+
{ owner, repo, issue_number, per_page: 100 },
91+
);
92+
// Strict login match: github-actions[bot] is the only
93+
// identity this workflow ever posts under. A broader
94+
// user.type === 'Bot' filter would let a third-party bot's
95+
// comment shadow ours and silently break the upsert.
96+
const matches = comments.filter(c =>
97+
c.user?.login === 'github-actions[bot]'
98+
&& typeof c.body === 'string'
99+
&& c.body.includes(marker),
100+
);
101+
// Update the most recent match and delete older
102+
// duplicates so a crashed prior run cannot leave stale
103+
// gate state visible on the PR.
104+
matches.sort((a, b) =>
105+
new Date(b.updated_at) - new Date(a.updated_at)
106+
);
107+
if (matches.length > 0) {
108+
await github.rest.issues.updateComment({
109+
owner, repo, comment_id: matches[0].id, body,
110+
});
111+
for (const stale of matches.slice(1)) {
112+
await github.rest.issues.deleteComment({
113+
owner, repo, comment_id: stale.id,
114+
});
115+
}
116+
} else {
117+
await github.rest.issues.createComment({
118+
owner, repo, issue_number, body,
119+
});
120+
}

configs/subsystem-budget.txt

Lines changed: 29 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -2,33 +2,35 @@
22
#
33
# Format: <bucket> <ceiling-bytes> [<noise-band-pct>]
44
#
5-
# - Bucket names match scripts/subsystem-rollup.py output. Run a
6-
# diagnostic build (KERNEL_DEBUG_INFO=reduced) and inspect
7-
# profiles/kernel-pgo/none/subsystem-rollup.txt for the live names.
8-
# - The noise band absorbs run-to-run variance from GCC LTO
9-
# re-deciding what to inline when nothing semantic changed. Default
10-
# is 2.0%. Start there, then tighten after observing a week of
11-
# clean builds. <icf-merged> tends to be jitterier than real
12-
# subsystems -- a wider band there is reasonable.
5+
# - Bucket names match scripts/subsystem-rollup.py output. The checker
6+
# now merges both subsystem-rollup.txt and subsystem-rollup-deep.txt,
7+
# so rules may target top-level buckets (`kernel`, `lib`, `fs`) or
8+
# deep sub-buckets (`kernel/sched`, `kernel/printk`, `lib/zstd`).
139
# - A breach is "actual > limit * (1 + band/100)". The total-bytes
14-
# gate is the coarse safeguard; this layer answers WHICH bucket
10+
# gate remains the coarse safeguard; this layer answers WHICH bucket
1511
# regressed.
12+
# - The default 2.0% band is still a placeholder until run-to-run LTO
13+
# variance is measured. `<icf-merged>` gets 5.0% because IPA-ICF
14+
# alias groups jitter more than the real subsystem buckets.
1615
#
17-
# How to populate:
18-
# 1. KERNEL_DEBUG_INFO=reduced ./build.sh linux bootwrapper
19-
# 2. Read profiles/kernel-pgo/none/subsystem-rollup.txt for the
20-
# observed sizes.
21-
# 3. Pick ceilings 5-10% above each observed value -- enough room
22-
# for legitimate growth without masking regressions.
23-
#
24-
# Example values (uncomment and tune to your build):
25-
# kernel 260000 2.0
26-
# mm 80000 2.0
27-
# fs 20000 2.0
28-
# arch/arm 120000 2.0
29-
# drivers/tty 25000 2.0
30-
# drivers/clocksource 10000 2.0
31-
# lib 70000 2.0
32-
# crypto 5000 2.0
33-
# <icf-merged> 30000 5.0
34-
# <compiler-partition> 5000 5.0
16+
# Baseline sources:
17+
# - Top-level ceilings come from the 2026-05-01 diagnostic snapshot in
18+
# TODO.md's "Snapshot from Latest Production Build" section.
19+
# - `kernel/sched` and `kernel/printk` come from the 2026-05-01
20+
# "Kernel and Lib Sub-Bucket Breakdown" section after patches
21+
# 0015-0020 and CONFIG_SCHED_FAIR_TINY.
22+
# - `lib/zstd`, `lib/lz4`, and `lib/xz` are pinned at zero to catch an
23+
# `olddefconfig` re-enable of already-disabled RD_* decompressors.
24+
25+
kernel 200000 2.0
26+
fs 165000 2.0
27+
lib 116000 2.0
28+
mm 86000 2.0
29+
<icf-merged> 28000 5.0
30+
31+
kernel/sched 23500 2.0
32+
kernel/printk 18500 2.0
33+
34+
lib/zstd 0 0.0
35+
lib/lz4 0 0.0
36+
lib/xz 0 0.0

scripts/check-subsystem-budget.py

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,10 +19,19 @@
1919

2020
import argparse
2121
import pathlib
22+
import re
2223
import sys
2324

2425
DEFAULT_BAND_PCT = 2.0
2526

27+
# The deep-rollup writer emits "# <bucket> top <N> source files" as the
28+
# delimiter that closes the depth-2 budget-eligible region. N is the
29+
# `top_files` parameter on subsystem-rollup.py:write_deep_table and is
30+
# not currently exposed as a CLI flag, so matching the literal "top 20"
31+
# would silently drift if that default changed. Match any positive int
32+
# instead so the two scripts stay decoupled.
33+
TOP_FILES_HEADER_RE = re.compile(r" top \d+ source files$")
34+
2635

2736
def read_budget(path):
2837
rules = {}
@@ -75,11 +84,52 @@ def read_rollup(path):
7584
return rows
7685

7786

87+
def read_deep_rollup(path):
88+
rows = {}
89+
in_depth2 = False
90+
for raw in path.read_text().splitlines():
91+
line = raw.strip()
92+
if not line:
93+
in_depth2 = False
94+
continue
95+
if line.startswith("## "):
96+
in_depth2 = False
97+
continue
98+
if line.startswith("# "):
99+
# Only the "by 2nd-level subdirectory" table is stable
100+
# budget material. The subsequent "top N source files"
101+
# table is intentionally excluded from the budget
102+
# namespace: file-level churn is too high, and a file name
103+
# like "kernel/workqueue.c" would collide conceptually
104+
# with a genuine depth-2 bucket if the format ever grows.
105+
if line.endswith(" by 2nd-level subdirectory"):
106+
in_depth2 = True
107+
elif TOP_FILES_HEADER_RE.search(line):
108+
in_depth2 = False
109+
continue
110+
if not in_depth2:
111+
continue
112+
parts = raw.split("\t")
113+
if len(parts) < 2:
114+
continue
115+
bucket = parts[0]
116+
try:
117+
rows[bucket] = int(parts[1])
118+
except ValueError:
119+
continue
120+
return rows
121+
122+
78123
def main(argv):
79124
ap = argparse.ArgumentParser(
80125
description="Compare subsystem rollup against per-bucket budgets."
81126
)
82127
ap.add_argument("--rollup", required=True, type=pathlib.Path)
128+
ap.add_argument(
129+
"--deep-rollup",
130+
type=pathlib.Path,
131+
help="Optional subsystem-rollup-deep.txt for sub-bucket rules.",
132+
)
83133
ap.add_argument("--budget", required=True, type=pathlib.Path)
84134
ap.add_argument(
85135
"--output",
@@ -101,9 +151,32 @@ def main(argv):
101151
file=sys.stderr,
102152
)
103153
return 2
154+
if args.deep_rollup is not None and not args.deep_rollup.exists():
155+
print(
156+
f"check-subsystem-budget: deep rollup not found: "
157+
f"{args.deep_rollup}",
158+
file=sys.stderr,
159+
)
160+
return 2
104161

105162
budgets = read_budget(args.budget)
106163
rollup = read_rollup(args.rollup)
164+
deep_rollup = (
165+
read_deep_rollup(args.deep_rollup)
166+
if args.deep_rollup is not None
167+
else {}
168+
)
169+
170+
overlap = sorted(set(rollup) & set(deep_rollup))
171+
if overlap:
172+
print(
173+
"check-subsystem-budget: top-level and deep rollups contain "
174+
"the same bucket name(s), refusing ambiguous input: "
175+
+ ", ".join(overlap),
176+
file=sys.stderr,
177+
)
178+
return 2
179+
rollup.update(deep_rollup)
107180

108181
if not budgets:
109182
# Empty file is a deliberate state: the operator has staged the
@@ -120,6 +193,11 @@ def main(argv):
120193
lines = [
121194
f"# subsystem budget check (default band = +/- {DEFAULT_BAND_PCT}%)",
122195
f"# rollup: {args.rollup}",
196+
(
197+
f"# deep rollup: {args.deep_rollup}"
198+
if args.deep_rollup is not None
199+
else "# deep rollup: (not provided)"
200+
),
123201
f"# budget: {args.budget}",
124202
"# bucket\tactual\tlimit\tband_pct\tdelta_vs_limit\tstatus",
125203
]

scripts/kernel-size-report.sh

Lines changed: 25 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -234,10 +234,33 @@ report_subsystem_budget() {
234234
return 0
235235
fi
236236

237-
python3 "${SUBSYSTEM_BUDGET_CHECK}" \
237+
# Wipe before the run so an exit other than 0/1 (missing input,
238+
# overlap-refusal, malformed file) cannot let a stale status file
239+
# from a cached workspace masquerade as the current gate state.
240+
rm -f "${OUTDIR}/subsystem-budget.txt"
241+
242+
set -- \
238243
--rollup "${OUTDIR}/subsystem-rollup.txt" \
239244
--budget "${SUBSYSTEM_BUDGET_FILE}" \
240-
--output "${OUTDIR}/subsystem-budget.txt" || true
245+
--output "${OUTDIR}/subsystem-budget.txt"
246+
if [ -f "${OUTDIR}/subsystem-rollup-deep.txt" ]; then
247+
set -- "$@" --deep-rollup "${OUTDIR}/subsystem-rollup-deep.txt"
248+
fi
249+
250+
rc=0
251+
python3 "${SUBSYSTEM_BUDGET_CHECK}" "$@" || rc=$?
252+
253+
# Exit codes from check-subsystem-budget.py:
254+
# 0 -- ok (status file written, gate clean)
255+
# 1 -- breach (status file written; warn-only at this layer)
256+
# 2 -- input (no status file; do not let a stale one survive)
257+
case "${rc}" in
258+
0|1) ;;
259+
*)
260+
echo "ERROR: subsystem budget checker exited ${rc}" >&2
261+
rm -f "${OUTDIR}/subsystem-budget.txt"
262+
;;
263+
esac
241264
}
242265

243266
case "${MODE}" in

0 commit comments

Comments
 (0)