Skip to content

ci: thin LTO + 16 codegen units + Defender exclusions for Windows publish - #4373

Merged
jqnatividad merged 2 commits into
masterfrom
windows-lto-tuning
Aug 9, 2026
Merged

ci: thin LTO + 16 codegen units + Defender exclusions for Windows publish#4373
jqnatividad merged 2 commits into
masterfrom
windows-lto-tuning

Conversation

@jqnatividad

Copy link
Copy Markdown
Collaborator

Items 3 and 5 from the Windows build-time list. Builds on #4370/#4371/#4372.

3. Thin LTO + 16 codegen units, Windows only

lto = true + codegen-units = 1 on a polars-sized binary is the most expensive possible setting, and the link phase is largely serial. Windows is the only platform where that cost has repeatedly failed to finish inside GitHub's 6h cap. Linux and macOS are untouched and keep fat LTO + codegen-units = 1.

This trades some runtime performance on the Windows binaries for a build that completes — noted in the changelog.

Why --config flags and not CARGO_PROFILE_* env vars

Verified empirically — an empty env var is a hard error, so there'd be no way to express "no override":

CARGO_PROFILE_RELEASE_LTO=""            → incorrect value `` for codegen option `lto`
CARGO_PROFILE_RELEASE_CODEGEN_UNITS=""  → cannot parse integer from empty string

An empty expression contributes no CLI argument at all. Both profile names (release, release-luau) are set because BIN_PROFILE picks one at runtime; defining an unused profile is harmless and mirrors what build-pgo.sh already does for its instrument build.

The msvc PGO optimize build gets the same treatment via the existing pgo-lto knob. codegen-units is not yet plumbed through build-pgo.sh — noted in a comment.

5. Windows Defender exclusions

Best-effort exclusions for the workspace, ~/.cargo, ~/.rustup, RUNNER_TEMP, plus the compiler processes. Real-time scanning of the thousands of object files and multi-GB LTO temporaries this build produces is a known Windows-CI tax.

continue-on-error with per-path try/catch: Defender may be policy-managed on the runner, and failing to set an exclusion must never fail a publish. This one is an unmeasured cheap experiment, not a known win — unlike item 3, I have no qsv-specific measurement for it.

Verification

Tested under bash, which is what CI uses. My first attempt tested under the local zsh and was invalid — zsh doesn't word-split unquoted variables, so it passed the whole tuning string as one argument and appeared broken:

case resulting command
tuned cargo build --profile release --config 'profile.release.lto="thin"' --config profile.release.codegen-units=16
empty cargo build --profile release — no stray argument

And the flags are effective, not merely acceptedcargo build -v shows -C lto=thin -C codegen-units=16 with the override, versus -C codegen-units=1 without.

  • actionlint: 0 findings on all three workflows, matching baseline. SC2086 on the intentional unquoted expansion is suppressed with an inline directive explaining why the word-splitting is required.
  • check-publish-matrix-sync.py and docs-drift-check.py pass.
  • The new lto/codegen-units keys are present on every matrix entry (empty on non-Windows) — actionlint rejects referencing a key that only some entries define, as it did for musl-prep in ci: reusable publish job + Windows-only publish, drop --emit=asm #4370.

🤖 Generated with Claude Code

…lish

Windows is the only platform where the release build has repeatedly failed to
finish inside GitHub's 6h job cap. `lto = true` + `codegen-units = 1` on a
polars-sized binary is the most expensive possible setting and the link phase is
largely serial, so Windows now trades some runtime performance for a build that
completes. Linux and macOS are untouched and keep fat LTO + codegen-units=1.

Passed as `--config` flags, NOT CARGO_PROFILE_*_{LTO,CODEGEN_UNITS} env vars.
That is deliberate and was verified: an EMPTY env var is a hard error -

  CARGO_PROFILE_RELEASE_LTO=""            -> incorrect value `` for codegen option `lto`
  CARGO_PROFILE_RELEASE_CODEGEN_UNITS=""  -> cannot parse integer from empty string

so there is no way to express "no override" that way, whereas an empty expression
contributes no CLI argument at all. Both profile names are set because
BIN_PROFILE picks one at runtime; defining an unused profile is harmless, and it
mirrors what build-pgo.sh already does for its instrument build.

The PGO optimize build on msvc gets the same treatment through the existing
pgo-lto knob. codegen-units is not yet plumbed through build-pgo.sh.

Also adds best-effort Windows Defender exclusions for the workspace, ~/.cargo,
~/.rustup and RUNNER_TEMP, plus the compiler processes. Real-time scanning of the
thousands of object files and multi-GB LTO temporaries a build like this produces
is a known Windows-CI tax. continue-on-error and per-path try/catch: Defender may
be policy-managed on the runner, and failing to set an exclusion must never fail
a publish. This one is an unmeasured cheap experiment, not a known win.

Verified under bash (CI uses shell: bash - an initial test under the local zsh
was invalid, since zsh does not word-split unquoted variables):
  - tuned   -> cargo build --profile release --config 'profile.release.lto="thin"'
               --config profile.release.codegen-units=16
  - empty   -> cargo build --profile release          (no stray argument)
and the flags are EFFECTIVE, not merely accepted: `cargo build -v` shows
`-C lto=thin -C codegen-units=16` with the override versus `-C codegen-units=1`
without.

shellcheck SC2086 on the unquoted expansion is suppressed with an inline
directive explaining why the word-splitting is required; actionlint stays at 0
findings on all three workflows, matching baseline.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@codacy-production

Copy link
Copy Markdown

Up to standards ✅

🟢 Issues 0 issues

Results:
0 new issues

View in Codacy

NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.

roborev 4133 (MEDIUM): `codegen-units: 16` is a YAML integer, but
publish-target.yml declares `codegen_units` as `type: string` and the callers
pass it straight through, so the Windows publish jobs could fail workflow_call
input validation before any build starts.

I could not confirm that failure mode. GitHub's workflow-syntax reference does
not state whether workflow_call inputs are strictly type-checked or coerced, and
actionlint - which does model matrix value types, and caught a real
`matrix.job.musl-prep` error earlier in this series - reported 0 findings on the
unquoted version.

Applied regardless: the declaration/value mismatch is real either way, quoting
costs nothing, and an unverifiable validation risk is not worth carrying on a
publish workflow that has already burned three multi-hour cycles.

Verified: actionlint 0 findings on all three workflows (matching baseline),
check-publish-matrix-sync.py and docs-drift-check.py pass. No Rust source is
touched by this change, so the cargo suite is not the relevant gate.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@jqnatividad
jqnatividad merged commit 997302e into master Aug 9, 2026
18 checks passed
@jqnatividad
jqnatividad deleted the windows-lto-tuning branch August 9, 2026 00:56
jqnatividad added a commit that referenced this pull request Aug 10, 2026
…orkflow to strip the portable binaries from its zip (#4382)

* docs(changelog): file the post-tag packaging work under 22.0.1, not Unreleased

The 22.0.1 release is still a pre-release, and fifteen commits of packaging and
publishing work landed after the tag to make it shippable. Both changelogs filed
that work under `[Unreleased]` — the section directly above `[22.0.1]` — so it read
as coming *after* the release it actually ships in.

Move those entries into each file's 22.0.1 section under a new "Packaging &
Publishing (post-tag)" subsection. The tag stays at 0dbd4c5, so the preamble says
plainly that these landed after it rather than implying the tag contains them, and
notes that a per-target asset refresh is still in flight — the x86_64-linux-gnu zip
predates the portable-binary removal by about six hours and still contains them.

Also corrects the synthesize/profile target list, which was wrong in both
directions: it claimed the x86_64 Windows targets had `profile` (they have neither
`profile` nor `synthesize`) and omitted aarch64-apple-darwin (which has both, via
the self-hosted macOS ARM64 workflows — publish.yml's entry for that target is
commented out). Verified against each publish workflow's feature list.

The 21.1.0...22.0.1 compare link only covers the tag, so a second SHA-pinned link
is added for the post-tag range rather than a moving master ref.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* docs: correct two stale viz_static claims that #4368 invalidated

#4368 dropped viz_static from the Windows publish targets, which left two places
asserting the opposite.

CHANGELOG.md's 22.0.1 viz entry claimed "Windows and macOS now compile the
viz_static tier". That is false for Windows at HEAD — rust-windows.yml builds
`...,ui,viz` only, because adding viz_static pushed the job from ~27min past its
90min timeout without ever reaching the tests. The entry contradicted the
"Windows prebuilts: viz yes, viz_static no" note in its own section. Narrowed to
macOS, with the Windows revert and its reason spelled out.

rust-windows.yml's own comment still justified itself with "Windows ships
viz_static in publish.yml" — the premise #4368 removed. The conclusion (no
Windows viz_static coverage) happens to still hold, so only the reasoning is
rewritten; the cargo test invocation is untouched.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* ci: add a manual workflow to strip portable binaries from a published zip

#4380 stopped publishing the portable binaries (qsvp, qsvplite, qsvpdp, qsvpmcp),
but assets uploaded before it landed still contain them — the 22.0.1
x86_64-unknown-linux-gnu zip among them. Rebuilding a target to drop four files is
disproportionate: that zip already postdates #4371/#4372/#4373, so the portable
binaries are its only discrepancy, and a rebuild would emit non-deterministic PGO
output differing from what users already downloaded.

The unsign -> modify -> re-sign flow mirrors macOS-arm64-selfhosted-publish-qsvpy.yml,
which already rewrites a published pre-release zip to add qsvpy binaries.

Scoped to x86_64-unknown-linux-gnu, with no target input, and running on ubuntu-24.04
to match the runner publish.yml builds that artifact on — glibc is forward
incompatible, so a binary built on 24.04 will not start on 22.04 and the smoke test
would fail on a good archive. The post-delete assertion requires qsv + qsvlite +
qsvdp + qsvmcp to survive, which holds only for this target: publish-target.yml skips
qsvdp for aarch64-unknown-linux-gnu and all non-Linux targets, and skips qsvmcp for
musl and aarch64-unknown-linux-gnu. The header records how to extend it properly.

Deletes with `7z d -tzip`, not Info-ZIP `zip -d`: publish-target.yml creates these
archives with `7z a -tzip`. Entries are resolved by exact basename rather than a
glob — a `qsv*` prefix match would take `qsv` itself with them.

Trust model, because this replaces a published artifact:
- the DOWNLOADED asset is verified against the release tag's public key before
  anything touches it. Without that anchor the workflow would strip whatever
  signature the asset carried (or none) and apply a fresh valid one, laundering a
  tampered artifact into a signed release asset — every later check only attests to
  the signature this workflow itself created
- the public key comes from the tag (`git show "$TAG:src/qsv-zipsign-public.key"`),
  not the branch: that is the key clients of that release embed via include_bytes!,
  so a rotation surfaces as a failure instead of false confidence
- the private key exists for exactly three steps — fetch, sign, shred. It is
  materialized after checkout, after the third-party downloader action, and after all
  archive manipulation, and destroyed before any archive content executes.
  persist-credentials: false keeps a git credential out of the same window
- the signature is re-verified after the smoke test, immediately before upload, so
  the bytes published are the bytes attested
- the tag must resolve to MAJOR.MINOR.PATCH, exist, and still be a PRERELEASE.
  Replacing an asset changes its sha256 and a stable release is likelier to have been
  pinned. Fails closed on an empty or unexpected isPrerelease value
- dry_run defaults to true; upload is the only gated step
- finding no portable binaries is a hard error, so an already-stripped archive is
  never re-signed and re-uploaded unchanged
- every keeper must survive and every victim must be gone before signing
- zipsign signs bytes, not zip structure, so a corrupted central directory could
  still verify: the rewritten qsv is extracted and run
- a constant concurrency group: it is evaluated from the raw input before the tag is
  resolved, so keying it on release_tag would let an auto-detect run and an explicit
  run for the same tag race anyway

Validated end-to-end locally with a throwaway zipsign keypair against a 7z-created
archive: an authentic asset completes the full flow, while a tampered asset and an
unsigned asset are both refused at the trust anchor, before any unsign. Also covers
the flat, nested and already-stripped layouts, and the prerelease guard against
prerelease/stable/missing/empty outcomes.

Note zipsign salts signatures with --context, defaulting to the input filename, so
the asset name must stay byte-identical throughout.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant