fix(codegen): let minsize choose ordinary shadow-root inlining - #10076
proggeramlug wants to merge 4 commits into
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (1)
Included review availability: Your plan provides up to 8 included reviews per hour; 5 remain after this review. 📝 WalkthroughWalkthroughThe compiler changes LLVM min-size inline attributes for shadow-root functions. New tests inspect emitted IR and runtime behavior across root modes, transports, optimization levels, and moving-GC schedules. Scoped CI installs the pinned Node.js version and prepares runtime providers. ChangesMin-size inline policy
Priority: ⬇️ Low Estimated code review effort: 3 (Moderate) | ~25 minutes Change: Bug fix Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant RegressionTest as standalone_minsize_inline_policy
participant Compiler as Perry compiler
participant LLVMIR as LLVM IR
participant Binary as compiled fixture
participant NodeOracle as Node oracle
RegressionTest->>NodeOracle: Run fixture and capture expected output
RegressionTest->>Compiler: Compile each root, transport, and optimization configuration
Compiler->>LLVMIR: Emit inline attributes and GC strategy
RegressionTest->>LLVMIR: Verify attributes and strategy
RegressionTest->>Binary: Run ordinary and moving-GC executions
RegressionTest->>NodeOracle: Compare output with expected output
Merge Risk: ⚪ Minimal · up to The updated min-size inlining policy and its CI preparation path have targeted coverage, with no actionable merge-blocking issue identified. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 56.25% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 16 functions across 5 files. (2 skipped: 2 unsupported.)
✨ Finishing Touches 💡 2📝 Generate docstrings 💡
🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
1 participant
Summary
Let LLVM's
minsizecost model decide ordinary shadow-root inlining under-Oz, instead of forcing every body admitted by the existing 8 KiB cap intoits callers. This does not emit
noinline: profitable inlining remains available.Keep normal speed profiles,
-Os, native-root hints, explicit hot hints,explicit noinline requests, and the separately admitted pre-statepoint path.
Both textual IR and native construction use the same definition-header renderer.
No new environment knob, runtime ABI, collector policy, or application dependency.
Independent coverage
visibility/rendering paths, and preserved explicit attributes.
callbacks, thrown-object identity, strings and allocation churn.
construction × shadow/native roots × Os/Oz × ordinary/moving GC (16 runs).
It asserts the intended IR policy and actual copying-GC counters, compares
exact output with the repository-pinned Node, and hides source during execution.
.node-version; runner Node is not an oracle.The fixture and harness do not require Claude Code or the surrounding application work.
Validation and remaining CI
92e2161c9built in 3m51s; the matching Wasm/providergraph built separately in 4m02s. Compiler and all nine archives were frozen
and hash-verified before/after native validation.
transports and root modes, Os/Oz, ordinary/moving execution. Every moving
arm reports 197 copying minors, 30,247 moved objects and 4,004 loop polls.
Retained record:
perry-minsize-inline-eh1Wkv/result.json.function attributes in numbered groups.
e13d47726fixes only the harness,retains all IR/liveness assertions, and adds inline/grouped/mixed plus
missing/malformed/duplicate-group controls. The successful rerun used this
corrected harness and the frozen
92e2161c9compiler, not a claimed new-headcompiler. Compiler/runtime source is identical between those commits.
all 16 native cases again): compiled directly with
rustc --testand explicitfrozen-compiler/runtime paths. This verifies the entry's behavior; it is not
represented as a Cargo CI run or a newly built
e13d47726compiler.freshness failure, two CI-only skips, including the rerun at
e13d47726.The earlier run skipped the compile tier. The current rerun completed all
script checks and the product warning check, then was deliberately stopped
because the remaining compile tier used the default target rather than the
dedicated cache. Its overall exit is 124; no full compile-tier pass is claimed.
behavior, then fails the new Oz header assertion, as intended.
both moving arms report 197 copying minors / 42,355 moved objects / 4,004
loop polls, and optimized IR proves actual retained call boundaries.
853,876 → 793,316 bytes (-60,560, 7.09%); all 12 emissions passed.
This is neither a full-executable percentage nor production native proof.
CI at
e13d47726(34606196285) passed check, warnings, both artifact buildsand the GC matrix. The scoped Linux job actually executed 495 passing tests
across 34 codegen suites, with no failures, ignored or filtered tests there.
The new native entry then failed before compiling or executing any fixture:
its runtime-preparation helper exhausted 600 seconds building a second,
perry-devprovider graph. This is a real CI setup defect, not an inheritedfailure or a native semantic result.
0f40434a2adds this suite to the existing coherent release-provider preparationbranch, outside the bounded fixture, and uses the existing checked prebuilt
protocol. It changes no compiler/runtime code and raises no timeout.
Seven setup tests pass, including actual-workflow branch selection with Cargo
stubbed, unrelated/partial-setup controls, missing-provider rejection, and build
failure propagation. The new selection test failed on the old workflow at
minsize_inline_policy, then passed with the correction.The complete native matrix also passed 16/16 again in explicit prebuilt
mode, with
CARGOpointing to a nonexistent executable to reject any secondbuild. The frozen
92e2161c9compiler and all nine archives remained unchanged;every moving case again reports 197 minors / 30,247 moved objects / 4,004 polls.
This is local macOS evidence, separate from the current-head Linux run below.
Record:
perry-minsize-inline-2jPNEG/result.json. Final script lint again has75 pass / one measured base freshness failure / two CI-only skips; compile
tier explicitly skipped.
The corrected current-head Linux native job 103305355243 passed at
15:29:02 UTC in run 34611922322.
It executed 496 passing tests across 35 Rust suites, with zero failures,
ignored or filtered tests, plus all seven runtime-setup tests. The actual
standalone_minsize_inline_policy_regressionentry passed in 265.09 seconds;that checked-in entry invokes and asserts the complete 16-case native matrix.
Rust suppresses successful test stdout, so the Linux log is not represented
as containing individual matrix rows or GC counter values. Its compiler was
built by current-head Cargo CI, unlike the separately identified local frozen
compiler. Retained log:
minsize-0f40434a2-ci-e2e-20260911.log.Current-head check and warnings also passed. Runtime again has 3,504 pass /
the same single base-main worker-stack failure / 4 ignored, and lint again
has only the measured base-main freshness failure. The GC matrix and aggregate
both passed (aggregate completed 15:37:39 UTC). All 12 printed failure/crash
diagnostics across all six current-head gap shards match exact base-main
603b074acrun34565492075; the new fixture passes shard 1. This comparesexit codes and printed excerpts, not full stdout. Retained comparison:
minsize-0f40434a2-ci-baseline-comparison-20260911.log.The separate zizmor finding is the unchanged
gate-failure-watch.yml:9trigger; workflow, script and zizmor configuration are byte-identical to base.
No all-CI-green claim; current-head CI failures are classified, and the
previous native setup failure is fixed with successful Linux execution.
The superseded
e13d47726run is completed/cancelled; its unfinished legacy GC stress is not a pass.
Other
e13d47726CI failures were compared with exact base-main603b074acrun
34565492075: runtime 3,504 pass / 1 fail / 4 ignored has the samenative_stack.rs:53 worker assertion and :60 unwrap; lint has the same public
freshness error. All 12 printed gap failure/crash diagnostics across all
six shards match that main run (exit codes and printed excerpts, not complete
stdout). The new
test_gap_minsize_inline_policypasses Linux shard 1.No all-CI-green claim. Local native validation is complete; pending CI alone
is not a reason to retain draft status under MERGE_GUIDE.
The superseded
92e2161c9CI run retainedalways()-conditioned jobs afternormal cancellation and held this PR's concurrency slot. Only that old run
was force-cancelled; its completed/cancelled status and the new head's running
jobs were verified. Earlier cancelled jobs are not a current-head verdict.
Includes
changelog.d/10076-minsize-inline-policy.md.No version bump is included.