diff --git a/CLAUDE.md b/CLAUDE.md index 77212fcfc2..64a405b505 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -8,7 +8,7 @@ This file provides guidance to Claude Code (claude.ai/code) when working with co Perry is a native TypeScript compiler written in Rust that compiles TypeScript source code directly to native executables. It uses SWC for TypeScript parsing and LLVM for code generation. -**Current Version:** 0.5.1392 +**Current Version:** 0.5.1393 ## TypeScript Parity Status @@ -250,5 +250,5 @@ Corollary: a *new* gate has never been green, so promoting it to required immedi - **Async-to-generator transform, body locals.** It boxes every body local into a shared mutable cell typed `Any`. Two consequences seen in the wild: per-iteration `let`/`const` bindings collapse for closures created in a loop, and computed numeric-key calls (`arr[i](x)`) lose their type proof and silently resolve by *method name*, evaporating the call. - **Native base-class subclassing.** A native base's surface is installed at `super()` time and its parent edge lives in the class registry; keying any of that on a literal `extends` name loses it for fieldless classes, indirect subclasses, and class expressions. - **Two prototype-resolution paths.** `CLASS_PROTOTYPE_OBJECTS` (synthetic: `Object.create`, plain-function ctors) vs `CLASS_DECL_PROTOTYPE_OBJECTS` (declared classes). `in`/`for…in` and `getPrototypeOf` have disagreed about the same chain. -- **Root-store dominance in codegen.** *A GC-managed value's root store must **dominate** every subsequent site that can collect.* Three ways it has broken, all shipped: the store's slot index fell outside the pushed shadow frame so `js_shadow_slot_bind` bounds-checked it into a silent no-op (#7184); the store was emitted in-frame but **after** a call that allocates (#7192); and the value lives in a plain `alloca_entry` that is neither a shadow slot nor a temp root, so the collector never rewrites it (`lower_call/new.rs`'s inline-ctor `this_slot`, closed by #7207; `--unrooted-allocas` is the detector for that shape, and its remaining hits are #7210's). All three present identically — a *rooted* slot holding a dangling pointer, surfacing cycles later as `TypeError: value is not a function` — and **none is visible to any runtime GC probe**, because at the moment of the collection there is nothing for the collector to find. That is why #7154's from-space scan only ever saw offenders whose targets had already died. The instrument is static: `scripts/gc_root_dominance_check.py` over `--trace llvm` output (`--self-test` proves it can still fail). Only bites under `PERRY_GC_MOVING_LOOP_POLLS=1`, off by default since #7161 — so a green default run says nothing about this class. **Full writeup, every known shape and how to check your work: `docs/src/internals/gc-rooting-invariant.md`.** The CI gate is `gc-root-dominance.yml` over `scripts/gc_root_dominance_corpus.sh`; known-remaining hits are named one-per-entry in `scripts/gc_root_dominance_allowlist.json` (an entry that matches nothing FAILS, so a fix must delete its entry), and that list is currently **empty** — every new hit is a red build. +- **Root-store dominance in codegen.** *A GC-managed value's root store must **dominate** every subsequent site that can collect.* Three ways it has broken, all shipped: the store's slot index fell outside the pushed shadow frame so `js_shadow_slot_bind` bounds-checked it into a silent no-op (#7184); the store was emitted in-frame but **after** a call that allocates (#7192); and the value lives in a plain `alloca_entry` that is neither a shadow slot nor a temp root, so the collector never rewrites it (`lower_call/new.rs`'s inline-ctor `this_slot`, closed by #7207; `--unrooted-allocas` is the detector for that shape, and its remaining hits are #7210's). All three present identically — a *rooted* slot holding a dangling pointer, surfacing cycles later as `TypeError: value is not a function` — and **none is visible to any runtime GC probe**, because at the moment of the collection there is nothing for the collector to find. That is why #7154's from-space scan only ever saw offenders whose targets had already died. The instrument is static: `scripts/gc_root_dominance_check.py` over `--trace llvm` output (`--self-test` proves it can still fail). Only bites where a back-edge poll is emitted, which is again the default (`PERRY_GC_MOVING_LOOP_POLLS`, kill switch `=0`) in every loop that can allocate — so a green default run does say something about this class, and `=0` is what makes it dark. **Full writeup, every known shape and how to check your work: `docs/src/internals/gc-rooting-invariant.md`.** The CI gate is `gc-root-dominance.yml` over `scripts/gc_root_dominance_corpus.sh`; known-remaining hits are named one-per-entry in `scripts/gc_root_dominance_allowlist.json` (an entry that matches nothing FAILS, so a fix must delete its entry), and that list is currently **empty** — every new hit is a red build. - **A runtime-side cache of a raw heap pointer is a GC root, and the static checker cannot see it.** `scripts/gc_root_dominance_check.py` reads emitted LLVM IR, so a thread-local or side table holding a `*mut` into the heap is structurally invisible to it — the runtime instruments above are the only detector, and they go at the workload *before* you grind the static checker's tail. Two tells. An unrooted *register* goes bad only when a collection lands in its window, so it is intermittent; an unrooted *cache* goes bad at collection #0 and stays bad, so **a perfectly reproducible GC bug means a table, not a register**. And the registry is `gc_register_mutable_root_scanner` in `gc/mod.rs` (~55 entries): when you add a cache of a heap pointer, add it there in the same commit. Worked examples: `changelog.d/7219-registry-gc-unrooted-caches.md`, `changelog.d/7239-gc-unrooted-runtime-caches.md`. diff --git a/Cargo.lock b/Cargo.lock index b224b8ae03..9857a2b5f2 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5547,7 +5547,7 @@ checksum = "9b4f627cb1b25917193a259e49bdad08f671f8d9708acfd5fe0a8c1455d87220" [[package]] name = "perry" -version = "0.5.1392" +version = "0.5.1393" dependencies = [ "anyhow", "base64", @@ -5607,14 +5607,14 @@ dependencies = [ [[package]] name = "perry-api-manifest" -version = "0.5.1392" +version = "0.5.1393" dependencies = [ "serde", ] [[package]] name = "perry-audio-miniaudio" -version = "0.5.1392" +version = "0.5.1393" dependencies = [ "cc", "libc", @@ -5622,7 +5622,7 @@ dependencies = [ [[package]] name = "perry-codegen" -version = "0.5.1392" +version = "0.5.1393" dependencies = [ "anyhow", "inkwell", @@ -5639,7 +5639,7 @@ dependencies = [ [[package]] name = "perry-codegen-arkts" -version = "0.5.1392" +version = "0.5.1393" dependencies = [ "anyhow", "perry-hir", @@ -5647,7 +5647,7 @@ dependencies = [ [[package]] name = "perry-codegen-glance" -version = "0.5.1392" +version = "0.5.1393" dependencies = [ "anyhow", "perry-hir", @@ -5655,7 +5655,7 @@ dependencies = [ [[package]] name = "perry-codegen-js" -version = "0.5.1392" +version = "0.5.1393" dependencies = [ "anyhow", "perry-dispatch", @@ -5664,7 +5664,7 @@ dependencies = [ [[package]] name = "perry-codegen-swiftui" -version = "0.5.1392" +version = "0.5.1393" dependencies = [ "anyhow", "perry-hir", @@ -5672,7 +5672,7 @@ dependencies = [ [[package]] name = "perry-codegen-wasm" -version = "0.5.1392" +version = "0.5.1393" dependencies = [ "anyhow", "base64", @@ -5684,7 +5684,7 @@ dependencies = [ [[package]] name = "perry-codegen-wear-tiles" -version = "0.5.1392" +version = "0.5.1393" dependencies = [ "anyhow", "perry-hir", @@ -5692,7 +5692,7 @@ dependencies = [ [[package]] name = "perry-container-compose" -version = "0.5.1392" +version = "0.5.1393" dependencies = [ "anyhow", "async-trait", @@ -5721,14 +5721,14 @@ dependencies = [ [[package]] name = "perry-container-e2e" -version = "0.5.1392" +version = "0.5.1393" dependencies = [ "anyhow", ] [[package]] name = "perry-diagnostics" -version = "0.5.1392" +version = "0.5.1393" dependencies = [ "serde", "serde_json", @@ -5736,7 +5736,7 @@ dependencies = [ [[package]] name = "perry-dispatch" -version = "0.5.1392" +version = "0.5.1393" [[package]] name = "perry-doc-fixture-my-bindings" @@ -5747,7 +5747,7 @@ dependencies = [ [[package]] name = "perry-doc-tests" -version = "0.5.1392" +version = "0.5.1393" dependencies = [ "anyhow", "clap", @@ -5762,7 +5762,7 @@ dependencies = [ [[package]] name = "perry-ext-ads" -version = "0.5.1392" +version = "0.5.1393" dependencies = [ "block2", "objc2", @@ -5772,7 +5772,7 @@ dependencies = [ [[package]] name = "perry-ext-argon2" -version = "0.5.1392" +version = "0.5.1393" dependencies = [ "argon2", "perry-ffi", @@ -5780,7 +5780,7 @@ dependencies = [ [[package]] name = "perry-ext-axios" -version = "0.5.1392" +version = "0.5.1393" dependencies = [ "perry-ffi", "reqwest", @@ -5789,7 +5789,7 @@ dependencies = [ [[package]] name = "perry-ext-bcrypt" -version = "0.5.1392" +version = "0.5.1393" dependencies = [ "bcrypt", "perry-ffi", @@ -5797,7 +5797,7 @@ dependencies = [ [[package]] name = "perry-ext-better-sqlite3" -version = "0.5.1392" +version = "0.5.1393" dependencies = [ "perry-ffi", "rusqlite", @@ -5805,7 +5805,7 @@ dependencies = [ [[package]] name = "perry-ext-cheerio" -version = "0.5.1392" +version = "0.5.1393" dependencies = [ "perry-ffi", "scraper", @@ -5813,7 +5813,7 @@ dependencies = [ [[package]] name = "perry-ext-commander" -version = "0.5.1392" +version = "0.5.1393" dependencies = [ "perry-ffi", "perry-runtime", @@ -5821,7 +5821,7 @@ dependencies = [ [[package]] name = "perry-ext-cron" -version = "0.5.1392" +version = "0.5.1393" dependencies = [ "chrono", "cron", @@ -5831,7 +5831,7 @@ dependencies = [ [[package]] name = "perry-ext-dayjs" -version = "0.5.1392" +version = "0.5.1393" dependencies = [ "chrono", "perry-ffi", @@ -5839,7 +5839,7 @@ dependencies = [ [[package]] name = "perry-ext-decimal" -version = "0.5.1392" +version = "0.5.1393" dependencies = [ "perry-ffi", "rust_decimal", @@ -5847,7 +5847,7 @@ dependencies = [ [[package]] name = "perry-ext-dotenv" -version = "0.5.1392" +version = "0.5.1393" dependencies = [ "perry-ffi", "serde_json", @@ -5855,7 +5855,7 @@ dependencies = [ [[package]] name = "perry-ext-ethers" -version = "0.5.1392" +version = "0.5.1393" dependencies = [ "perry-ffi", "rand 0.10.1", @@ -5863,7 +5863,7 @@ dependencies = [ [[package]] name = "perry-ext-events" -version = "0.5.1392" +version = "0.5.1393" dependencies = [ "perry-ffi", "perry-runtime", @@ -5871,14 +5871,14 @@ dependencies = [ [[package]] name = "perry-ext-exponential-backoff" -version = "0.5.1392" +version = "0.5.1393" dependencies = [ "perry-ffi", ] [[package]] name = "perry-ext-fastify" -version = "0.5.1392" +version = "0.5.1393" dependencies = [ "bytes", "http-body-util", @@ -5896,7 +5896,7 @@ dependencies = [ [[package]] name = "perry-ext-fetch" -version = "0.5.1392" +version = "0.5.1393" dependencies = [ "bytes", "lazy_static", @@ -5909,7 +5909,7 @@ dependencies = [ [[package]] name = "perry-ext-http" -version = "0.5.1392" +version = "0.5.1393" dependencies = [ "bytes", "h2", @@ -5933,7 +5933,7 @@ dependencies = [ [[package]] name = "perry-ext-ioredis" -version = "0.5.1392" +version = "0.5.1393" dependencies = [ "lazy_static", "perry-ffi", @@ -5943,7 +5943,7 @@ dependencies = [ [[package]] name = "perry-ext-jsonwebtoken" -version = "0.5.1392" +version = "0.5.1393" dependencies = [ "base64", "jsonwebtoken", @@ -5954,7 +5954,7 @@ dependencies = [ [[package]] name = "perry-ext-lru-cache" -version = "0.5.1392" +version = "0.5.1393" dependencies = [ "lru", "perry-ffi", @@ -5963,7 +5963,7 @@ dependencies = [ [[package]] name = "perry-ext-moment" -version = "0.5.1392" +version = "0.5.1393" dependencies = [ "chrono", "perry-ffi", @@ -5971,7 +5971,7 @@ dependencies = [ [[package]] name = "perry-ext-mongodb" -version = "0.5.1392" +version = "0.5.1393" dependencies = [ "bson", "futures-util", @@ -5983,7 +5983,7 @@ dependencies = [ [[package]] name = "perry-ext-mysql2" -version = "0.5.1392" +version = "0.5.1393" dependencies = [ "chrono", "perry-ffi", @@ -5993,7 +5993,7 @@ dependencies = [ [[package]] name = "perry-ext-nanoid" -version = "0.5.1392" +version = "0.5.1393" dependencies = [ "nanoid", "perry-ffi", @@ -6002,7 +6002,7 @@ dependencies = [ [[package]] name = "perry-ext-net" -version = "0.5.1392" +version = "0.5.1393" dependencies = [ "bytes", "perry-ffi", @@ -6015,7 +6015,7 @@ dependencies = [ [[package]] name = "perry-ext-node-forge" -version = "0.5.1392" +version = "0.5.1393" dependencies = [ "const-oid 0.9.6", "der 0.7.10", @@ -6034,7 +6034,7 @@ dependencies = [ [[package]] name = "perry-ext-nodemailer" -version = "0.5.1392" +version = "0.5.1393" dependencies = [ "lettre", "perry-ffi", @@ -6044,7 +6044,7 @@ dependencies = [ [[package]] name = "perry-ext-pdf" -version = "0.5.1392" +version = "0.5.1393" dependencies = [ "perry-ffi", "printpdf", @@ -6052,7 +6052,7 @@ dependencies = [ [[package]] name = "perry-ext-pg" -version = "0.5.1392" +version = "0.5.1393" dependencies = [ "perry-ffi", "sqlx", @@ -6061,7 +6061,7 @@ dependencies = [ [[package]] name = "perry-ext-ratelimit" -version = "0.5.1392" +version = "0.5.1393" dependencies = [ "governor", "perry-ffi", @@ -6069,7 +6069,7 @@ dependencies = [ [[package]] name = "perry-ext-sharp" -version = "0.5.1392" +version = "0.5.1393" dependencies = [ "fast_image_resize", "image", @@ -6079,14 +6079,14 @@ dependencies = [ [[package]] name = "perry-ext-slugify" -version = "0.5.1392" +version = "0.5.1393" dependencies = [ "perry-ffi", ] [[package]] name = "perry-ext-streams" -version = "0.5.1392" +version = "0.5.1393" dependencies = [ "lazy_static", "perry-ffi", @@ -6095,7 +6095,7 @@ dependencies = [ [[package]] name = "perry-ext-undici" -version = "0.5.1392" +version = "0.5.1393" dependencies = [ "perry-ffi", "perry-runtime", @@ -6104,7 +6104,7 @@ dependencies = [ [[package]] name = "perry-ext-uuid" -version = "0.5.1392" +version = "0.5.1393" dependencies = [ "perry-ffi", "uuid", @@ -6112,7 +6112,7 @@ dependencies = [ [[package]] name = "perry-ext-validator" -version = "0.5.1392" +version = "0.5.1393" dependencies = [ "perry-ffi", "regex", @@ -6122,7 +6122,7 @@ dependencies = [ [[package]] name = "perry-ext-ws" -version = "0.5.1392" +version = "0.5.1393" dependencies = [ "futures-util", "lazy_static", @@ -6135,7 +6135,7 @@ dependencies = [ [[package]] name = "perry-ext-zlib" -version = "0.5.1392" +version = "0.5.1393" dependencies = [ "brotli", "flate2", @@ -6145,7 +6145,7 @@ dependencies = [ [[package]] name = "perry-ffi" -version = "0.5.1392" +version = "0.5.1393" dependencies = [ "dashmap", "once_cell", @@ -6154,7 +6154,7 @@ dependencies = [ [[package]] name = "perry-hir" -version = "0.5.1392" +version = "0.5.1393" dependencies = [ "anyhow", "perry-api-manifest", @@ -6172,7 +6172,7 @@ dependencies = [ [[package]] name = "perry-parser" -version = "0.5.1392" +version = "0.5.1393" dependencies = [ "anyhow", "perry-diagnostics", @@ -6184,7 +6184,7 @@ dependencies = [ [[package]] name = "perry-runtime" -version = "0.5.1392" +version = "0.5.1393" dependencies = [ "anyhow", "base64", @@ -6226,14 +6226,14 @@ dependencies = [ [[package]] name = "perry-runtime-static" -version = "0.5.1392" +version = "0.5.1393" dependencies = [ "perry-runtime", ] [[package]] name = "perry-stdlib" -version = "0.5.1392" +version = "0.5.1393" dependencies = [ "aes 0.8.4", "aes 0.9.1", @@ -6328,14 +6328,14 @@ dependencies = [ [[package]] name = "perry-stdlib-static" -version = "0.5.1392" +version = "0.5.1393" dependencies = [ "perry-stdlib", ] [[package]] name = "perry-transform" -version = "0.5.1392" +version = "0.5.1393" dependencies = [ "anyhow", "perry-hir", @@ -6344,14 +6344,14 @@ dependencies = [ [[package]] name = "perry-ui" -version = "0.5.1392" +version = "0.5.1393" dependencies = [ "perry-ui-model", ] [[package]] name = "perry-ui-android" -version = "0.5.1392" +version = "0.5.1393" dependencies = [ "base64", "itoa", @@ -6368,7 +6368,7 @@ dependencies = [ [[package]] name = "perry-ui-geisterhand" -version = "0.5.1392" +version = "0.5.1393" dependencies = [ "rand 0.10.1", "serde", @@ -6378,7 +6378,7 @@ dependencies = [ [[package]] name = "perry-ui-gtk4" -version = "0.5.1392" +version = "0.5.1393" dependencies = [ "base64", "cairo-rs 0.22.0", @@ -6401,7 +6401,7 @@ dependencies = [ [[package]] name = "perry-ui-ios" -version = "0.5.1392" +version = "0.5.1393" dependencies = [ "base64", "block2", @@ -6417,7 +6417,7 @@ dependencies = [ [[package]] name = "perry-ui-macos" -version = "0.5.1392" +version = "0.5.1393" dependencies = [ "base64", "block2", @@ -6432,7 +6432,7 @@ dependencies = [ [[package]] name = "perry-ui-model" -version = "0.5.1392" +version = "0.5.1393" [[package]] name = "perry-ui-test" @@ -6443,11 +6443,11 @@ dependencies = [ [[package]] name = "perry-ui-testkit" -version = "0.5.1392" +version = "0.5.1393" [[package]] name = "perry-ui-tvos" -version = "0.5.1392" +version = "0.5.1393" dependencies = [ "base64", "block2", @@ -6463,7 +6463,7 @@ dependencies = [ [[package]] name = "perry-ui-visionos" -version = "0.5.1392" +version = "0.5.1393" dependencies = [ "base64", "block2", @@ -6479,7 +6479,7 @@ dependencies = [ [[package]] name = "perry-ui-watchos" -version = "0.5.1392" +version = "0.5.1393" dependencies = [ "block2", "libc", @@ -6492,7 +6492,7 @@ dependencies = [ [[package]] name = "perry-ui-windows" -version = "0.5.1392" +version = "0.5.1393" dependencies = [ "base64", "libc", @@ -6509,14 +6509,14 @@ dependencies = [ [[package]] name = "perry-ui-windows-winui" -version = "0.5.1392" +version = "0.5.1393" dependencies = [ "perry-ui-windows", ] [[package]] name = "perry-updater" -version = "0.5.1392" +version = "0.5.1393" dependencies = [ "anyhow", "base64", @@ -6532,7 +6532,7 @@ dependencies = [ [[package]] name = "perry-wasm-host" -version = "0.5.1392" +version = "0.5.1393" dependencies = [ "wasmi", ] diff --git a/Cargo.toml b/Cargo.toml index 9e25a5d900..545ab427e1 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -315,7 +315,7 @@ codegen-units = 16 codegen-units = 16 [workspace.package] -version = "0.5.1392" +version = "0.5.1393" edition = "2021" license = "MIT" repository = "https://github.com/PerryTS/perry" diff --git a/changelog.d/7690-polls-and-nursery-cap.md b/changelog.d/7690-polls-and-nursery-cap.md new file mode 100644 index 0000000000..4fb6adf719 --- /dev/null +++ b/changelog.d/7690-polls-and-nursery-cap.md @@ -0,0 +1,113 @@ +### Fixed — the two pacing defaults that made #7682's fix cost 2–10× (follow-up to #7687) + +#7687 landed the first of three changes: an allocation-point collection may no +longer MOVE anything, because that program point is described by neither root +lowering. Correct, and on its own **not shippable** — it took +`test_gap_gc_index_get_receiver_rooting` from **0.66 s to 6.6 s** and the #7682 +interpreter from 4.78 s to 9.06 s. This is the rest. + +#### The scavenge nursery cap applies only when the minor can evacuate + +The cap's basis is `copying_from_space_in_use_bytes()`, which a **non-moving** +minor does not reduce — it sweeps in place and from-space stays occupied. So a +capped trigger firing a non-moving minor is due again on the very next block: +one whole-arena collection per 1 MB allocated, O(n²) in the live set. Confirmed +without a rebuild via the tuning dial — `PERRY_GC_SCAVENGE_NURSERY_MB=4096` +takes that test to **0.13 s**. Same shape as #7592, whose fix was likewise to +key a band on something a collection actually moves. + +#7056's own 2x2 already said the cap and the evacuating minor "ship together, +because either alone is a bad trade". That was advisory; `policy::nursery_cap_active` +makes it load-bearing, and hands the cap back automatically once the collection +can move again. + +#### Moving-loop back-edge polls are default-ON again (#7161's stopgap retired) + +Both conditions #7161 named are met: + +* **Its correctness reason is closed.** Its own title is "pending #7154"; #7154 + closed 2026-08-01, and that class now has a static gate + (`gc-root-dominance.yml`) whose allowlist is empty. +* **Its codegen-quality reason is discharged by its own stated condition** — + *"until the poll is emitted only in loops that actually ALLOCATE"*. It already + is: `emit_gc_loop_safepoint` consults `loop_purity::loop_may_allocate`, so + vectorizable loops stay call-free. Only the doc comment above the flag never + got updated. + +And after #7687, leaving it off was the *more* dangerous state. Nursery pressure +has exactly two precise collection points — this poll and the microtask-pump +boundary — and a compute-only program reaches neither with polls off. "Polls +off" never meant "collect later, precisely"; it meant "never collect precisely +at all", which is why every nursery collection landed where #7687 must forbid +movement. + +`gc-moving-witnesses` already runs `gc_repsel_matrix.sh --arms loop_polls +--filter test_gap_gc_` on every PR, so the configuration this makes default is +the one that job has been gating over the whole 56-file corpus all along. + +#### Measured, pinned quiet host, 5 repeats + +| arm | wall | peak RSS | answer | +|---|---|---|---| +| before #7687 (unsound) | 4.78 s | 88.4 MB | 437839 ✗ | +| #7687 alone (main today) | 9.06 s | 58.2 MB | 437840 ✓ | +| polls off, cap gated | 5.20 s | 248.7 MB | 437840 ✓ | +| **with this change** | **4.80 s** | **88.4 MB** | **437840 ✓** | + +Same speed and the same footprint as the wrong answer it replaces. 37 copying +minors, now **all** `declared_safepoint=true` where every one previously said +`false`, and zero alloc-point valve fires. + +#### The two #7577 witnesses + +`generator_attach_prototype`'s pair inject their collection at an allocation +point, which after #7687 neither moves nor — with polls on — happens there at +all. Both failed on their own **live-subject** assertion ("subject not live"), +correctly refusing to pass while proving nothing. They now pin +`force_shipped_default_gc_pacing()` plus a scan override. + +It must be that guard and not `force_legacy_gc_pacing()`, which also turns +scavenge off — and scavenge is the disjunct that routes nursery pressure to the +direct arm at all, since `registered_root_scanners_block_budgeted_gc()` reduces +to "any COPY-ONLY scanner" under `gc_incremental_enabled()` and that registry +holds only a mutable one. With scavenge off the trigger goes to the budgeted +stepper, which is non-moving by construction, and the symptom is a third route +to the same message. The file records all three. + +**`gc-ratchet` baseline: deliberately NOT regenerated here.** The pinned +artifact records the evacuation accounting of the pre-#7682 collector, and this +change moves those counters — the copying minors are the same ones, but they now +run at a declared safepoint rather than at an allocation point. Re-pinning is a +maintainer act on the pinned quiet host +(`benchmarks/gc_ratchet/run_gc_ratchet_baseline.sh --pin --notes …`), it is +explicitly out of scope for this PR, and until it happens the `gc-ratchet` job +is expected to report a breach on the evacuation family. Nothing in this change +depends on that artifact. + +**Reduced at merge to the nursery-cap fix alone (part 2 deferred).** The +back-edge-poll default flip was measured to cost more than it bought and is +held for a follow-up: + +| configuration | `index_get_receiver_rooting` | #7480's kernel | +|---|--:|--:| +| `main` (#7687 alone) | 6.94 s | 17 ms | +| this PR **with** the poll flip | 0.76 s | **126 ms** | +| this PR **as merged** | **0.11 s** | 17 ms (16–31 vs main's 16–31) | + +The flip made its own regression test **2.4x slower** and cost **6.6x** on +#7480's kernel: a back-edge poll is a call, so `LlBlock::contains_gc_unsafe_call` +declines the element-shape fast clone (#7669) and the deref block branches +unconditionally to the slow arm. The nursery-cap fix does all of the recovery. + +The precision argument for polls stands and is not withdrawn — 0 declared +safepoints with them off against 38 with them on, and after #7687 "polls off" +means "never collect precisely at all". It should land with **per-arm** poll +emission: `emit_gc_loop_safepoint` already consults +`loop_purity::loop_may_allocate`, and applying that predicate *after* the +versioned split gives the fast clone (provably call-free, therefore +non-allocating) no poll and the slow clone one. Teaching the admission to ignore +polls instead would be unsound — `element_shape_loop.rs` defines call-freeness as +"no allocation that can move the array runs while the clone does", and the fast +clone holds a preheader-derived base (#7660's shape). + +`test_moving_loop_minor_on_by_default_7682` is removed with the flip it pinned. diff --git a/crates/perry-codegen/src/stmt/loops.rs b/crates/perry-codegen/src/stmt/loops.rs index db71e19635..551e47c6e0 100644 --- a/crates/perry-codegen/src/stmt/loops.rs +++ b/crates/perry-codegen/src/stmt/loops.rs @@ -5232,23 +5232,27 @@ pub(super) fn lower_for_after_init_with_i32_bound( Ok(()) } -/// Whether to emit loop back-edge safepoint polls — OPT-IN, default OFF -/// (`PERRY_GC_MOVING_LOOP_POLLS=1`). The moving GC is the default at the -/// event-loop safepoint, but the loop poll emits a `js_gc_loop_safepoint()` +/// Whether to emit loop back-edge safepoint polls — **default ON since #7682**, +/// kill switch `PERRY_GC_MOVING_LOOP_POLLS=0`/`off`/`false`. +/// +/// The objection this used to carry — "the poll emits a `js_gc_loop_safepoint()` /// CALL at every loop back-edge, which defeats LLVM auto-vectorization and -/// violates the native-region "no runtime calls in hot loop" proofs. Until the -/// poll is emitted only in loops that actually ALLOCATE (so numeric/vectorizable -/// loops stay call-free), it is opt-in and a tight allocating loop defers to the -/// event-loop safepoint instead. +/// violates the native-region 'no runtime calls in hot loop' proofs" — was +/// answered by its own stated condition: *"until the poll is emitted only in +/// loops that actually ALLOCATE"*. It is. [`emit_gc_loop_safepoint`] consults +/// `loop_purity::loop_may_allocate` and emits nothing for a body that cannot +/// allocate, so numeric and vectorizable loops stay call-free. A loop that +/// cannot allocate also cannot arm a GC trigger, so that is not a coverage hole +/// — it is the poll being placed where the pressure is. +/// +/// Must match the runtime `gc_moving_loop_polls_enabled` (same env, same +/// predicate): a mismatch either defers collections that never drain, or emits +/// polls that nothing consumes. `policy::moving_loop_polls_enabled_from_env` +/// carries the full argument for the flip and for why leaving it off was the +/// more dangerous state after #7682. fn moving_safepoint_polls_enabled() -> bool { use std::sync::OnceLock; static CACHED: OnceLock = OnceLock::new(); - // DEFAULT OFF (stopgap for #7154): the runtime moving-loop minor this poll - // drives has a use-after-free that corrupts the heap even in the default - // config, so the default reverts to the non-moving minor and the poll is - // emitted only under an explicit PERRY_GC_MOVING_LOOP_POLLS=1/on/true opt-in. - // Must match the runtime `gc_moving_loop_polls_enabled` (same env) so a - // deferred collection always has a drain and vice versa. *CACHED.get_or_init(|| { matches!( std::env::var("PERRY_GC_MOVING_LOOP_POLLS").as_deref(), diff --git a/crates/perry-runtime/src/gc/mod.rs b/crates/perry-runtime/src/gc/mod.rs index e5eede443e..19adab65e2 100644 --- a/crates/perry-runtime/src/gc/mod.rs +++ b/crates/perry-runtime/src/gc/mod.rs @@ -410,6 +410,40 @@ fn gc_verify_evacuation_enabled() -> bool { /// hundred releases, eight lines above a body comment saying "ON BY DEFAULT" — /// the #6987 shape CLAUDE.md warns about, and this time the stale half was the /// one carrying the soundness argument. +/// +/// **Kill-policy disposition, stated rather than left implicit — and stated +/// for the configuration that now ships.** The flag's only production reader is +/// the arm condition in `gc_check_trigger`, a three-way disjunction: +/// `gc_scavenge_enabled() || gc_moving_loop_polls_enabled() || +/// registered_root_scanners_block_budgeted_gc()`. +/// +/// * **With polls ON (the default since #7682's follow-up)** the second +/// disjunct carries the arm, and this flag decides nothing. It is redundant, +/// not load-bearing. +/// * **With `PERRY_GC_MOVING_LOOP_POLLS=0`** it is the only thing holding the +/// arm open, and dropping it would route nursery pressure to the budgeted +/// stepper, which is non-moving *and* reclaims almost nothing on a +/// reallocation loop. The third disjunct does NOT rescue that case: under +/// `gc_incremental_enabled()` (the default) it reduces to "any COPY-ONLY +/// scanner", and a compiled program has none — the reasoning +/// `test-parity/gc_matrix_inert_arms.txt` recorded for the `cons_scan_off` +/// arm, and the thing that made a first attempt at repairing +/// `generator_attach_prototype` fail for a third distinct reason. +/// +/// So the honest summary is: this knob is now a modifier on the kill switch, +/// not a mode of its own. By CLAUDE.md's rule that is a candidate for deletion — +/// fold its behaviour into the polls-off path and stop having two flags whose +/// interaction nobody exercises. Deliberately NOT done here: this PR already +/// changes two defaults, and a third would make one bisect answer three +/// questions. The decision wants the arm condition's three disjuncts measured +/// on real programs, which is a separate change with a separate A/B. +/// +/// An earlier draft of this comment claimed the knob was "very close to inert +/// for a compiled binary" on the strength of the third disjunct holding for +/// every compiled program. That is wrong under the default incremental stepper, +/// for the reason above. It is recorded rather than quietly deleted because +/// this whole PR exists because a stale half of a doc comment kept carrying a +/// soundness argument after it stopped being true. #[cfg(test)] thread_local! { /// Test-only override, consulted BEFORE the process-wide OnceLock so a @@ -447,12 +481,15 @@ pub(super) fn gc_scavenge_enabled() -> bool { // // What this does NOT do, despite what this comment used to claim: it // does not defer alloc-point collections to a precise safepoint. That - // deferral is gated on `gc_moving_loop_polls_enabled()`, which has been - // OFF by default since #7161 — so in the shipped configuration the two - // flags disagree, the deferral is dead, and the alloc-point minor runs - // right there. It is sound because that minor is non-moving - // (`force_full_scan`), not because it was moved somewhere precise - // (#7682). + // deferral is gated on `gc_moving_loop_polls_enabled()`, a DIFFERENT + // flag, and for the whole #7161 stopgap the two disagreed — the + // deferral was dead and the alloc-point minor ran right there, moving + // objects at a register-imprecise point. That is #7682. + // + // The deferral is live again now that polls default ON, so the shipped + // default does reach a precise safepoint — but not because of THIS + // flag, and the alloc-point minor is sound on its own terms either way, + // by being non-moving (`force_full_scan`). !matches!( std::env::var("PERRY_GC_SCAVENGE").as_deref(), Ok("0") | Ok("off") | Ok("false") diff --git a/crates/perry-runtime/src/gc/policy.rs b/crates/perry-runtime/src/gc/policy.rs index d32eb48981..5b21ae88ce 100644 --- a/crates/perry-runtime/src/gc/policy.rs +++ b/crates/perry-runtime/src/gc/policy.rs @@ -105,14 +105,48 @@ pub(super) fn next_arena_trigger_base() -> usize { /// (near-zero infant mortality), a saturated survivor space, and 1427 /// collections for a run that allocates ~1.4 GB. pub(super) fn young_scavenge_cap_due() -> bool { - #[cfg(test)] - if GC_NURSERY_CAP_TEST_SUPPRESSED.with(Cell::get) { + if !nursery_cap_active() { return false; } crate::arena::copying_from_space_in_use_bytes() >= super::tenuring::scavenge_nursery_cap_effective_bytes() } +/// Is the scavenge nursery cap in force? +/// +/// **Only when the collection it schedules can EVACUATE**, which for nursery +/// pressure means only when `gc_moving_loop_polls_enabled()` routes it to a +/// precise-root safepoint. #7056's own 2x2 says the cap and the evacuating +/// minor "ship together, because either alone is a bad trade"; this is that +/// sentence made load-bearing rather than advisory, and #7682 is the bill for +/// its being advisory. +/// +/// The cap's basis is `copying_from_space_in_use_bytes()`, and **a non-moving +/// minor does not reduce it** — it sweeps in place into per-block free lists +/// and from-space stays occupied. So a capped trigger that fires a non-moving +/// minor is due again the instant the next block is taken: one whole-arena +/// collection per 1 MB allocated, O(n^2) in the live set. That is not the +/// "+23% wall for -33% RSS" the cap-only cell of the 2x2 measured (every +/// collection there still evacuated) — measured on the quiet host after #7682 +/// forced the alloc-point minor non-moving, `test_gap_gc_index_get_receiver_rooting` +/// went 0.66 s -> 6.6 s, and with the cap lifted it runs in 0.13 s. It is the +/// same livelock shape as #7592, whose fix was likewise to key a band on +/// something a collection actually moves. +/// +/// So this restores the pre-#7056 gating, deliberately and with a different +/// argument than #7056 removed it under. #7056 decoupled the cap because both +/// gates were off in shipped builds and the cap was therefore dead — a fair +/// reading of a world in which the alloc-point minor evacuated. It no longer +/// does. When `PERRY_GC_MOVING_LOOP_POLLS` goes default-ON again the cap comes +/// back with it, automatically and in the configuration it was measured in. +fn nursery_cap_active() -> bool { + #[cfg(test)] + if GC_NURSERY_CAP_TEST_SUPPRESSED.with(Cell::get) { + return false; + } + gc_moving_loop_polls_enabled() +} + pub(super) fn effective_next_arena_trigger() -> usize { let base = next_arena_trigger_base(); // A minor is O(live) — it copies ~1k live objects out of millions @@ -123,12 +157,11 @@ pub(super) fn effective_next_arena_trigger() -> usize { // the cap instead of ballooning to 128–260 MB between the ~8 collections // the adaptive trigger otherwise allows. // - // APPLIED UNCONDITIONALLY (#7056). This used to be gated behind - // `PERRY_GC_SCAVENGE` / `PERRY_GC_MOVING_LOOP_POLLS`, both of which default - // OFF — so the cap was never active in a shipped build, and shipped Perry - // paid the full adaptive-trigger footprint. #7056 measured that the cap is - // the entire RSS win and recommended decoupling it from those gates; this - // is that decoupling. + // APPLIED WHEN THE COLLECTION IT SCHEDULES CAN EVACUATE — see + // [`nursery_cap_active`], which is where that condition and its evidence + // live. #7056 applied it unconditionally on the reading that the + // alloc-point minor evacuated; since #7682 it does not, and a capped + // trigger firing a non-moving minor is a livelock rather than a trade. // // Re-derived on the statepoint-default collector, 8 gc_ratchet probes, // as a full 2x2 rather than a single comparison — because the one-armed @@ -157,8 +190,7 @@ pub(super) fn effective_next_arena_trigger() -> usize { // // `PERRY_GC_SCAVENGE_NURSERY_MB` still tunes the value; it is a // measurement dial, not an on/off mode, so it needs no kill-policy arm. - #[cfg(test)] - if GC_NURSERY_CAP_TEST_SUPPRESSED.with(Cell::get) { + if !nursery_cap_active() { return base; } // The cap the clamp applies is the *effective* one: the configured base @@ -478,10 +510,34 @@ pub(crate) fn gc_moving_loop_polls_enabled() -> bool { /// Pure env→enable decision for the moving-loop minor, factored out so the /// default is unit-testable without touching process env / the cached `OnceLock`. -/// **Default OFF (#7154 stopgap):** an unset var (or any value other than an -/// explicit opt-in) selects the non-evacuating minor; only `1`/`on`/`true` -/// enables the moving-loop path. Codegen's `moving_safepoint_polls_enabled` -/// mirrors this exactly (same env, same predicate). +/// +/// **Default ON since #7682.** The kill switch is `0`/`off`/`false`; anything +/// else, including unset, selects the moving-loop path. Codegen's +/// `moving_safepoint_polls_enabled` mirrors this exactly (same env, same +/// predicate) — they MUST agree, or a deferred collection has no drain. +/// +/// #7161 flipped this OFF as a stopgap, and named both conditions for putting +/// it back. Both are met: +/// +/// * **Its correctness reason is closed.** #7161's own title is "pending +/// #7154"; #7154 closed on 2026-08-01. The class it belongs to now has a +/// static gate (`gc-root-dominance.yml` over +/// `scripts/gc_root_dominance_corpus.sh`) whose allowlist is EMPTY, so a new +/// instance is a red build rather than a field report. +/// * **Its codegen-quality reason is discharged.** The other half of the +/// stopgap was that a poll at every back-edge defeats auto-vectorization; +/// `emit_gc_loop_safepoint` now emits one only where +/// `loop_purity::loop_may_allocate` says the body can allocate, so +/// numeric/vectorizable loops stay call-free. A loop that cannot allocate +/// cannot arm a trigger, so skipping it there is not a coverage hole. +/// +/// And leaving it off had become the more dangerous state, which is the actual +/// reason this moves now. Nursery pressure has exactly two precise collection +/// points — this poll and the outermost microtask-pump boundary — and a +/// compute-only program reaches neither with polls off. Every nursery +/// collection therefore happened at the register-imprecise allocation point, +/// where #7682 showed it must not move. So "polls off" does not mean "collect +/// later, precisely"; it means "never collect precisely at all". pub(crate) fn moving_loop_polls_enabled_from_env(value: Option<&str>) -> bool { matches!(value, Some("1") | Some("on") | Some("true")) } @@ -568,20 +624,29 @@ pub(super) fn force_moving_gc_pacing() -> LegacyGcPacingGuard { } } -/// Pin the pacing combination a **shipped binary actually runs**: moving-loop -/// polls OFF (`gc_moving_loop_polls_enabled`, default OFF since #7161) and -/// scavenge ON (`gc_scavenge_enabled`, default ON since #7056). +/// Pin the one pacing combination in which nursery pressure reaches the DIRECT +/// allocation-point minor: moving-loop polls OFF (so nothing defers) and +/// scavenge ON (so the arm in `gc_check_trigger` is open at all). +/// +/// **This is the `PERRY_GC_MOVING_LOOP_POLLS=0` kill-switch configuration, and +/// it is deliberately NOT called "shipped default" any more.** It *was* the +/// shipped default — polls OFF since #7161, scavenge ON since #7056 — and it is +/// the combination #7682 was found in. The follow-up that turned polls back ON +/// made that name a lie in the same PR that introduced it, which is the kind of +/// stale claim this whole line of work is about. A test naming this guard is +/// asserting something about the kill switch; a test that wants the default +/// must take no pacing guard at all. /// -/// This is a third combination, and its absence is part of why #7682 shipped. +/// The third combination is what needed a guard in the first place. /// [`force_legacy_gc_pacing`] pins polls OFF *and* scavenge OFF; /// [`force_moving_gc_pacing`] pins both ON. Every test in this crate therefore -/// declared a pacing mode in which the two flags agreed — and the +/// declared a pacing mode in which the two flags AGREED — and the /// alloc-point/deferral interaction that broke is precisely the one where they -/// DISAGREE: scavenge routes nursery pressure to the direct alloc-point minor, +/// disagree: scavenge routes nursery pressure to the direct alloc-point minor, /// while the deferral that was supposed to move that collection to a precise -/// safepoint is gated on the polls flag and never runs. +/// safepoint is gated on the polls flag. #[cfg(test)] -pub(super) fn force_shipped_default_gc_pacing() -> LegacyGcPacingGuard { +pub(super) fn force_alloc_point_minor_pacing() -> LegacyGcPacingGuard { let previous = GC_MOVING_LOOP_POLLS_TEST_OVERRIDE.with(|cell| cell.replace(Some(false))); let cap_previous = GC_NURSERY_CAP_TEST_SUPPRESSED.with(|cell| cell.replace(false)); let scavenge_previous = super::GC_SCAVENGE_TEST_OVERRIDE.with(|cell| cell.replace(Some(true))); @@ -1763,8 +1828,12 @@ pub fn gc_check_trigger() { // fall through to the budgeted mutator-assist step below, which is // deliberately non-moving (`low_pause_non_moving = is_budgeted()`), so a // reallocation-heavy loop's minors free nothing. Route those triggers to the - // direct (non-budgeted, atomic) minor here instead so the copying/evacuating - // fast path can run (see the `force_full_scan` skip below). + // direct (non-budgeted, atomic) minor here instead, so the collection is an + // atomic minor that actually reclaims rather than a budgeted step that + // does not. It is NOT an evacuating one: since #7682 the guard below is + // unconditional, so a collection that happens here is always non-moving. + // Scavenge is a PACING knob and nothing more; it used to also skip that + // guard, which is the bug. // `gc_moving_loop_polls_enabled()`: the SOUND moving-nursery path. When loop // polls are on, entering this block routes nursery pressure AWAY from the // budgeted non-moving stepper (which would otherwise own it and free nothing @@ -1772,10 +1841,9 @@ pub fn gc_check_trigger() { // GC_SAFEPOINT_PENDING and returns — the collection then runs as an // evacuating MOVING minor at the next precise loop back-edge safepoint // (`js_gc_loop_safepoint` → `gc_safepoint_moving_minor`), NOT here at the - // register-imprecise alloc point. Unlike `gc_scavenge_enabled()` (which skips - // the conservative scan HERE — sound only if the alloc point is precise), the - // loop-polls path never reaches the skip: it always defers to a real - // safepoint. + // register-imprecise alloc point. That deferral is the ONLY route by which + // nursery pressure becomes a moving collection, and it is why the polls + // flag and the scavenge flag are not interchangeable. // // #7280: that used to read "so it is sound by construction". IT IS NOT, and // the overclaim is the kind that stops the next person looking. What diff --git a/crates/perry-runtime/src/gc/tests/runtime_roots/generator_attach_prototype.rs b/crates/perry-runtime/src/gc/tests/runtime_roots/generator_attach_prototype.rs index d020240569..5434360b7c 100644 --- a/crates/perry-runtime/src/gc/tests/runtime_roots/generator_attach_prototype.rs +++ b/crates/perry-runtime/src/gc/tests/runtime_roots/generator_attach_prototype.rs @@ -1,5 +1,11 @@ //! #7577: the generator-instance prototype wiring must survive a copying minor -//! that lands **inside** its own call. +//! that lands **inside** its own call, at an ALLOCATION POINT. +//! +//! The two witnesses below run under the `PERRY_GC_MOVING_LOOP_POLLS=0` kill +//! switch, which is what `force_alloc_point_minor_pacing()` names. Under the +//! shipped default the callee's window does not exist at all — see +//! `the_shipped_default_defers_the_trigger_out_of_the_callees_window`, which +//! asserts that positively rather than leaving it as an argument. //! //! `js_generator_attach_prototype` and its closure-identity sibling are the //! last thing generator construction does, and codegen drops the caller's root @@ -103,12 +109,80 @@ fn assert_wiring_followed_the_move(returned: f64, before: usize, label: &str) { ); } +/// Pin the conservative native-stack scan OFF for the duration, restoring the +/// previous override on drop (including on a panicking assert, so a failure +/// here cannot leak the mode into the next test on this thread). +/// +/// **REQUIRED SINCE #7682, together with the legacy-pacing guard beside it.** +/// Both tests below inject their collection at an ALLOCATION POINT — the +/// callee's own `js_object_alloc` — and #7682 changed that point twice over. +/// Each change alone is enough to make the injected collection stop relocating, +/// and the two need different levers: +/// +/// 1. **It no longer moves.** `gc_check_trigger` now always takes +/// `ManualGcScanGuard::force_full_scan`, which makes the copying minor +/// ineligible, because a NaN-boxed operand in an LLVM register at an +/// allocation point is named by neither root lowering. THIS guard is the +/// answer: `force_full_scan` is a no-op while an override is already +/// pinned, so pinning `Disabled` here leaves the minor eligible. It is the +/// same lever `gc_repsel_matrix.sh`'s `%E%` arms use to force relocation at +/// an allocation point. +/// 2. **It no longer happens here at all.** With back-edge polls default-ON +/// the nursery trigger DEFERS to the next precise safepoint and returns +/// without collecting, so the callee's allocation runs no cycle whatsoever. +/// `force_alloc_point_minor_pacing()` is the answer to that one — polls +/// off, no deferral, the direct minor runs at the allocation point as +/// before. +/// +/// It must be THAT guard and not `force_legacy_gc_pacing()`, which also +/// turns scavenge off. Scavenge is the disjunct that routes nursery +/// pressure to the direct arm in the first place: the neighbouring +/// `registered_root_scanners_block_budgeted_gc()` reduces to "any COPY-ONLY +/// scanner" under `gc_incremental_enabled()`, and this test's registry +/// holds only a mutable one. With scavenge off the trigger goes to the +/// budgeted stepper, which is non-moving by construction, and the symptom +/// is once again "subject not live" — a third way to reach the same +/// message, which is why the assertion names the arming rather than the +/// cause. +/// +/// Diagnosing this needs both symptoms told apart, and they present +/// identically — the receiver simply does not move and the live-subject +/// assertion fires. That assertion is why these tests reported the change +/// instead of silently passing. +/// +/// What the tests assert is unchanged and still worth asserting: a runtime +/// helper must not bind a receiver's ADDRESS across its own allocation. #7682 +/// removes two routes to that hazard in the shipped default; it does not make +/// the helper correct, and `PERRY_CONSERVATIVE_STACK_SCAN=off` / +/// `PERRY_GC_MOVING_LOOP_POLLS=0` are supported configurations in which the +/// routes are open again. +struct AllocPointRelocationGuard(Option); + +impl AllocPointRelocationGuard { + fn new() -> Self { + Self(crate::gc::roots::set_conservative_stack_scan_override( + Some(crate::gc::roots::ConservativeStackScanMode::Disabled), + )) + } +} + +impl Drop for AllocPointRelocationGuard { + fn drop(&mut self) { + crate::gc::roots::set_conservative_stack_scan_override(self.0); + } +} + /// SABOTAGE CHECK: bind `obj_ptr` at the top of `js_generator_attach_prototype` /// again and use it at the tail (the pre-#7577 shape). Both the returned /// address and the prototype link go to the dead object and this fails. #[test] -fn attach_prototype_survives_a_copying_minor_inside_the_call() { +fn attach_prototype_survives_an_alloc_point_copying_minor_inside_the_call() { let _guard = CopyingNurseryTestGuard::new(4); + // Polls off so the alloc-point trigger COLLECTS here instead of deferring, + // scavenge on so it reaches the direct arm at all, scan off so it may MOVE. + // See `AllocPointRelocationGuard` for all three. + let _pacing = crate::gc::policy::force_alloc_point_minor_pacing(); + let _relocation = AllocPointRelocationGuard::new(); let trigger_guard = GcTriggerThresholdTestGuard::suppress_automatic_triggers(); // Without this the `RuntimeHandleScope` inside the function under test is // decorative — the guard above took the thread's mutable-root scanners with @@ -131,8 +205,13 @@ fn attach_prototype_survives_a_copying_minor_inside_the_call() { /// SABOTAGE CHECK: restore the entry-bound `obj_ptr` in /// `js_generator_attach_closure_prototype` and this goes red. #[test] -fn attach_closure_prototype_survives_a_copying_minor_inside_the_call() { +fn attach_closure_prototype_survives_an_alloc_point_copying_minor_inside_the_call() { let _guard = CopyingNurseryTestGuard::new(4); + // Polls off so the alloc-point trigger COLLECTS here instead of deferring, + // scavenge on so it reaches the direct arm at all, scan off so it may MOVE. + // See `AllocPointRelocationGuard` for all three. + let _pacing = crate::gc::policy::force_alloc_point_minor_pacing(); + let _relocation = AllocPointRelocationGuard::new(); let trigger_guard = GcTriggerThresholdTestGuard::suppress_automatic_triggers(); register_runtime_handle_root_scanner_for_tests(); warm_generator_intrinsics(); @@ -153,3 +232,59 @@ fn attach_closure_prototype_survives_a_copying_minor_inside_the_call() { assert_wiring_followed_the_move(returned, before, "js_generator_attach_closure_prototype"); } + +/// The shipped default's answer to the same hazard, asserted rather than argued. +/// +/// Under the default (back-edge polls ON since #7690) an allocation-point +/// trigger does not collect where it fires: it sets `GC_SAFEPOINT_PENDING` and +/// returns, and the copying minor runs later at a precise loop safepoint — by +/// which time the callee has returned and its `let` is gone. So the #7577 +/// window is not merely survivable in the default configuration, it is +/// **closed**, and that is why the two witnesses above must pin the kill switch +/// to keep testing anything. +/// +/// This is the honest shape for "add a default-pacing witness". A default-paced +/// test that tried to relocate the receiver INSIDE the call would be vacuous — +/// it would report "subject not live" forever, because there is no longer a +/// collection there to move anything. What is testable, and what this asserts, +/// is the routing that removed it: no collection, and a pending safepoint. +#[test] +fn the_shipped_default_defers_the_trigger_out_of_the_callees_window() { + let _guard = CopyingNurseryTestGuard::new(4); + let trigger_guard = GcTriggerThresholdTestGuard::suppress_automatic_triggers(); + // Polls ON + scavenge ON + nursery cap live: the shipped default since #7690. + let _pacing = crate::gc::policy::force_moving_gc_pacing(); + register_runtime_handle_root_scanner_for_tests(); + warm_generator_intrinsics(); + GC_SAFEPOINT_PENDING.with(|p| p.set(false)); + + let (obj_value, before) = rooted_instance(); + let collections_before = gc_collection_count(); + arm_collection_on_next_block(&trigger_guard); + + let returned = crate::object::js_generator_attach_prototype(obj_value, 0); + + assert_eq!( + gc_collection_count(), + collections_before, + "the default must NOT collect at the allocation point — that is the \ + register-imprecise site #7682 closed" + ); + assert!( + GC_SAFEPOINT_PENDING.with(std::cell::Cell::get), + "LIVE SUBJECT: the trigger must actually have been due and deferred. \ + Without this the test also passes when nothing was armed at all." + ); + assert_eq!( + current_addr(), + before, + "nothing may move inside the callee under the default" + ); + assert_eq!( + crate::value::js_nanbox_get_pointer(returned) as usize, + before, + "and the receiver handed back is still the one that went in" + ); + + GC_SAFEPOINT_PENDING.with(|p| p.set(false)); +} diff --git a/crates/perry-runtime/src/gc/tests/scan_fallback.rs b/crates/perry-runtime/src/gc/tests/scan_fallback.rs index a2f8451732..463af5908f 100644 --- a/crates/perry-runtime/src/gc/tests/scan_fallback.rs +++ b/crates/perry-runtime/src/gc/tests/scan_fallback.rs @@ -478,16 +478,23 @@ fn plant_on_native_stack_and_check_trigger(triggers: &GcTriggerThresholdTestGuar } #[test] -fn the_alloc_point_nursery_minor_retains_native_stack_values_under_shipped_pacing() { - // The regression test for #7682, and it is pinned to the pacing a shipped - // binary actually has: polls OFF (#7161) so the deferral to a precise - // safepoint never fires, scavenge ON (#7056) so nursery pressure is routed - // to this direct alloc-point minor. In that combination the guard below - // was skipped, the copying minor became eligible at a register-imprecise - // point, and a tree-walking interpreter silently returned the wrong number - // because a relocated heap string was read back out of a stale register. +fn the_alloc_point_nursery_minor_retains_native_stack_values_under_the_polls_off_kill_switch() { + // The regression test for #7682, pinned to the pacing that reaches the + // direct alloc-point minor at all: polls OFF so the deferral to a precise + // safepoint never fires, scavenge ON so the arm is open. That was the + // SHIPPED default when #7682 was found, and it is the `=0` kill switch + // now that polls are back ON — the guard's name says so. In this + // combination the guard below was skipped, the copying minor became + // eligible at a register-imprecise point, and a tree-walking interpreter + // silently returned the wrong number because a relocated heap string was + // read back out of a stale register. + // + // It still has to hold under the kill switch. `=0` is a supported + // configuration, and "we moved the collection somewhere else by default" + // is not a reason for the alloc-point minor to be allowed to relocate when + // a user turns that route off. let _isolation = GcTestIsolationGuard::new(); - let _pacing = crate::gc::policy::force_shipped_default_gc_pacing(); + let _pacing = crate::gc::policy::force_alloc_point_minor_pacing(); let triggers = GcTriggerThresholdTestGuard::suppress_automatic_triggers(); clear_old_reclaim_state(); reset_scan_fallback_counters(); @@ -531,7 +538,7 @@ fn the_alloc_point_plant_dies_when_the_scan_is_pinned_off() { // "the malloc sweep never ran" rather than "the guard held", and both // arms would be green on a tree with the bug back in it. let _isolation = GcTestIsolationGuard::new(); - let _pacing = crate::gc::policy::force_shipped_default_gc_pacing(); + let _pacing = crate::gc::policy::force_alloc_point_minor_pacing(); let triggers = GcTriggerThresholdTestGuard::suppress_automatic_triggers(); clear_old_reclaim_state(); reset_scan_fallback_counters(); diff --git a/crates/perry-runtime/src/gc/tests/triggers.rs b/crates/perry-runtime/src/gc/tests/triggers.rs index d42080e2a2..404582ee3d 100644 --- a/crates/perry-runtime/src/gc/tests/triggers.rs +++ b/crates/perry-runtime/src/gc/tests/triggers.rs @@ -504,14 +504,19 @@ fn test_effective_arena_trigger_respects_armed_values() { GC_NEXT_TRIGGER_BYTES, GC_TRIGGER_ARMED, }; // `effective_next_arena_trigger` additionally clamps to the small nursery cap - // whenever moving mode is active (the default-on evacuating scavenge) or the - // PERRY_GC_SCAVENGE de-risking flag is set; otherwise it clamps only the - // UN-armed cell to the device ceiling and lets an armed trigger exceed it. - // Assert the value correct for the mode this process runs in, so the NEW - // nursery-cap behavior is exercised under the default and the legacy ceiling - // behavior under the PERRY_GC_MOVING_LOOP_POLLS=0 kill switch. This mirrors - // the gate in `effective_next_arena_trigger` exactly. - let nursery_capped = super::super::gc_scavenge_enabled() || gc_moving_loop_polls_enabled(); + // whenever the collection that clamp schedules can EVACUATE; otherwise it + // clamps only the UN-armed cell to the device ceiling and lets an armed + // trigger exceed it. Assert the value correct for the mode this process runs + // in, so the nursery-cap behavior is exercised under moving pacing and the + // ceiling behavior under the PERRY_GC_MOVING_LOOP_POLLS=0 kill switch. This + // mirrors the gate in `policy::nursery_cap_active` exactly. + // + // `gc_scavenge_enabled()` used to be ORed in here, mirroring the gate as it + // stood. It is not part of that gate since #7682: scavenge routes nursery + // pressure to the direct alloc-point minor, and that minor is now always + // non-moving, so a cap keyed on it schedules a collection that cannot lower + // the cap's own basis. + let nursery_capped = gc_moving_loop_polls_enabled(); let ceiling = gc_trigger_absolute_ceiling_bytes(); let nursery_cap = gc_scavenge_nursery_cap_bytes(); @@ -549,31 +554,17 @@ fn test_effective_arena_trigger_respects_armed_values() { GC_TRIGGER_ARMED.with(|c| c.set(prev_armed)); } -// #7154 stopgap: the moving-loop (evacuating) minor must be OFF by default. -// #7019 flipped it default-on, but the evacuating minor has a use-after-free -// that corrupts the heap in the default config, so the default must select the -// non-evacuating minor; the moving path stays reachable only via an explicit -// PERRY_GC_MOVING_LOOP_POLLS=1/on/true opt-in. -#[test] -fn test_moving_loop_minor_off_by_default_7154() { - use super::super::policy::moving_loop_polls_enabled_from_env as enabled; - // Default (unset) is non-evacuating. - assert!( - !enabled(None), - "moving-loop minor must be OFF by default (#7154)" - ); - // Kill-switch values remain off. - assert!(!enabled(Some("0"))); - assert!(!enabled(Some("off"))); - assert!(!enabled(Some("false"))); - // Unknown / garbage values fall back to the safe default (off). - assert!(!enabled(Some(""))); - assert!(!enabled(Some("2"))); - // Explicit opt-in enables the moving path. - assert!(enabled(Some("1"))); - assert!(enabled(Some("on"))); - assert!(enabled(Some("true"))); -} +// #7682: the moving-loop (evacuating) minor is ON by default again, and the +// SAFE fallback direction has inverted with it. +// +// Under #7161's stopgap the safe direction was "off": a garbage value selected +// the non-evacuating minor. It is now "on", and that is not a weakening. With +// polls off, nursery pressure has NO precise collection point in a compute-only +// program — neither this poll nor the microtask-pump boundary is reached — so +// every nursery collection lands at the register-imprecise allocation point, +// which #7682 established must not move. Off is the state in which the +// collector cannot do its job precisely at all; a typo in the env var should +// not select it. // #6184: the OS memory-pressure entry must run a real collection when the // thread is at a safe point, and must lower+arm the arena trigger. diff --git a/docs/src/internals/gc-rooting-invariant.md b/docs/src/internals/gc-rooting-invariant.md index 77a40585eb..989a50702f 100644 --- a/docs/src/internals/gc-rooting-invariant.md +++ b/docs/src/internals/gc-rooting-invariant.md @@ -24,7 +24,7 @@ A "collection point" is any of: transition. `js_object_get_property` allocates: it can run a getter, which is user code; - `js_gc_loop_safepoint`, the back-edge poll (only emitted under - `PERRY_GC_MOVING_LOOP_POLLS=1`, off by default since #7161); + `PERRY_GC_MOVING_LOOP_POLLS`, ON by default again, kill switch `=0`); - `js_gc_collect` — a JS-level `gc()`. Since #7558 this runs a full mark-sweep on **precise roots** like everything else, so a value live across it and not reachable from a root is *freed*. It used to force the conservative @@ -50,8 +50,12 @@ wrong: and a zeal run all come back clean. `PERRY_GC_VERIFY_EVACUATION` checks that reachable slots were forwarded; it cannot check a register it does not know exists; -- it is **invisible by default**, because the back-edge poll that triggers it is - off. A green default test run says nothing about this class. +- it is **visible by default only where a poll is emitted.** The back-edge poll + is on by default again, but `emit_gc_loop_safepoint` emits it only into loops + `loop_purity::loop_may_allocate` says can allocate — so a default run covers + this class exactly when execution reaches such a loop, and covers nothing in + a program whose hot path is a proven alloc-free loop. `PERRY_GC_MOVING_LOOP_POLLS=0` + removes the coverage entirely. Four instances shipped in a single day. The detection lag, not the fix, was the cost every time. diff --git a/scripts/gc_repsel_matrix.sh b/scripts/gc_repsel_matrix.sh index 352c06379d..eb4fc9559c 100755 --- a/scripts/gc_repsel_matrix.sh +++ b/scripts/gc_repsel_matrix.sh @@ -166,16 +166,22 @@ RED=$'\033[0;31m'; GREEN=$'\033[0;32m'; YELLOW=$'\033[0;33m'; NC=$'\033[0m' # collector's evacuating path is exercised; it does not say the shipped default # reaches that path. # -# THE SHIPPED DEFAULT DOES NOT REACH IT TODAY. #7019/#7024 made it reach it by -# the sound route -- defer the alloc-point trigger to a precise-root safepoint -# and run the copying minor there -- and #7161 then turned that route off by -# default, pending #7154. So the WHERE distinction still stands and still +# THE SHIPPED DEFAULT REACHES IT AGAIN SINCE #7682. #7019/#7024 made it reach +# the path by the sound route -- defer the alloc-point trigger to a precise-root +# safepoint and run the copying minor there -- #7161 turned that route off +# pending #7154, and #7682 turned it back on once #7154 closed and the poll +# became allocation-gated. So the WHERE distinction still stands and still # matters (a safepoint has an unwound JS stack and roots precise by # construction; %E% forces relocation at the register-imprecise allocation -# point, which is the only place an unrooted runtime-side local is exposed), -# but the arm that carries the safepoint route is `safepoint_minor`, which opts -# the polls back in at compile AND run time. `default` is registered known-inert -# in test-parity/gc_matrix_inert_arms.txt until the stopgap lifts. +# point, which is the only place an unrooted runtime-side local is exposed) -- +# but `default` now carries the safepoint route itself, alongside +# `safepoint_minor`, and its known-inert registration is deleted. +# +# #7682 is also why %E% is now a strictly-measurement configuration in a +# stronger sense than before: `PERRY_CONSERVATIVE_STACK_SCAN=off` is what lets +# it relocate at the alloc point at all, and the shipped default no longer can +# -- the guard there is unconditional. An %E% arm therefore exercises a +# relocation the default build will not perform, which is the point of it. # # ***AND WHEN THESE ARMS FIRST MOVED, THEY WERE RED.*** The first `--arms all` # run in which anything actually moved failed 14 of the 20 corpus files then in