From 5c692d264b1de4864df45d65ca79d6f99921bceb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Tue, 11 Aug 2026 03:42:45 +0200 Subject: [PATCH 1/2] =?UTF-8?q?fix(ci):=20unbreak=20the=20thread-local=20p?= =?UTF-8?q?olicy=20gate=20=E2=80=94=20convert=20the=20two=20live=20declara?= =?UTF-8?q?tions,=20stop=20counting=20`#[cfg(test)]`=20ones?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `scripts/check_thread_locals.py` has been red on `main` (exit 1). Because it is deliberately not yet a required context, nothing surfaced it — and that red state is exactly what blocks `tls-budget`'s documented promotion step, so the gate stays advisory indefinitely. This is the shape CLAUDE.md calls gate theatre: it runs, it reports failure, and it cannot block anything. TWO OF THE EIGHT RAW BLOCKS ARE REAL, AND THEY ARE NOT THE ONES REPORTED. #7814 names `array/indexing.rs`, `map.rs`, `set.rs` and `registry_latch_probes.rs` as "hot path" violations. All four are `#[cfg(test)]`: they do not exist in a shipping build and cannot cost a single `_tlv_get_addr`. The genuinely live declarations are in `gc/schedule.rs`, which the ticket does not mention — `SAFEPOINT_COUNTER` (bumped on every safepoint) and `SCHEDULE_NEXT_CANDIDATE_BYTES` (read on every poll). Those two are converted to `crate::perry_thread_local!`. `#[cfg(test)]` DECLARATIONS ARE OUT OF SCOPE BY CONSTRUCTION. Recording a test-only declaration as "cold" records the wrong fact — it is not cold, it is absent — and spends the allowlist's credibility on entries nobody can ever act on. The scan now skips three statically-visible gated shapes: the attribute directly above a block, an inline `#[cfg(test)] mod … { … }`, and a file whose whole module is declared `#[cfg(test)] mod ;` (closed transitively, so `gc/tests/mod.rs` carries its subtree). That is the direction that can go wrong quietly — an over-broad exclusion makes the gate pass by seeing less — so `--self-test` grew two cases per shape: gated is not counted, AND removing the gate makes the same declaration fail again. It now proves the checker can fail in six directions, not four. The allowlist regeneration is provably one-way: 101 -> 91 entries, 157 -> 129 blocks, with nothing added and no entry's count increased. Three of the dropped entries were already stale (`gc/zeal.rs` from #7741's removal of `PERRY_GC_ZEAL`, plus `arena/quarantine.rs` and `gc/oldgen_defrag.rs`). `_hot_declarations` also moves 157 -> 160, and only two of those three are mine: the recorded value was already one behind the tree, since nothing compares it except the capacity ceiling. Still a maintainer action, and the point of the exercise: promote `tls-budget` to a required context after its first green run on `main`. Refs #7814 --- crates/perry-runtime/src/gc/schedule.rs | 4 +- scripts/check_thread_locals.py | 147 ++++++++++++++++++++++- scripts/thread_local_cold_allowlist.json | 32 ++--- 3 files changed, 157 insertions(+), 26 deletions(-) diff --git a/crates/perry-runtime/src/gc/schedule.rs b/crates/perry-runtime/src/gc/schedule.rs index e5addfde94..b645095f03 100644 --- a/crates/perry-runtime/src/gc/schedule.rs +++ b/crates/perry-runtime/src/gc/schedule.rs @@ -129,7 +129,7 @@ static SCHEDULE_FORCED: AtomicU64 = AtomicU64::new(0); /// other test's `before + 1` flaky. static SCHEDULE_SAFEPOINTS: AtomicU64 = AtomicU64::new(0); -thread_local! { +crate::perry_thread_local! { /// The monotonically increasing safepoint ordinal this thread's schedule is /// a function of. Thread-local on purpose — see the determinism note above. static SAFEPOINT_COUNTER: std::cell::Cell = const { std::cell::Cell::new(0) }; @@ -512,7 +512,7 @@ impl Drop for ScheduleStrideGuard { } } -thread_local! { +crate::perry_thread_local! { /// From-space high-water mark at or above which the next poll-arm candidate /// is due. Per-thread because the arena it measures is. /// diff --git a/scripts/check_thread_locals.py b/scripts/check_thread_locals.py index 6b801c6656..a5039ac990 100755 --- a/scripts/check_thread_locals.py +++ b/scripts/check_thread_locals.py @@ -32,6 +32,24 @@ * `--self-test` runs both directions against synthetic trees, so the checker itself cannot quietly stop being able to say no. +`#[cfg(test)]` DECLARATIONS ARE OUT OF SCOPE BY CONSTRUCTION +=========================================================== + +A `thread_local!` that only exists under `#[cfg(test)]` cannot cost +`_tlv_get_addr` in a shipping build, because it is not in one. Recording such a +declaration as "cold" would be recording the wrong fact — it is not cold, it is +*absent* — and it would spend the allowlist's credibility on entries no one can +ever act on. Three gated shapes are therefore not counted at all: + +* `#[cfg(test)]` immediately above the block, +* the block nested inside a `#[cfg(test)] mod … { … }` in the same file, +* the block's whole FILE being a test module, i.e. some `lib.rs`/`mod.rs` + declares it `#[cfg(test)] mod ;`. + +This is a *static* fact the scan can see, unlike hot-vs-cold. It is also the +direction that can go wrong quietly, so `--self-test` checks that removing the +gate makes the same declaration fail again. + WHAT IT DOES NOT DO =================== @@ -77,6 +95,17 @@ # macro without infinite regress. EXCLUDED = {"crates/perry-runtime/src/tls_hot.rs"} +# `#[cfg(test)] mod ;` — the whole file is a test module. +CFG_TEST_MOD_RE = re.compile( + r"(?m)^[ \t]*#\[cfg\(test\)\]\s*\n[ \t]*(?:pub(?:\([^)]*\))?\s+)?mod\s+([A-Za-z_0-9]+)\s*;" +) +# Any out-of-line `mod ;`, gated or not — the edges of the module tree. +ANY_MOD_RE = re.compile(r"(?m)^[ \t]*(?:pub(?:\([^)]*\))?\s+)?mod\s+([A-Za-z_0-9]+)\s*;") +# `#[cfg(test)] mod {` — an inline test module, whose body is skipped. +CFG_TEST_INLINE_MOD_RE = re.compile( + r"(?m)^[ \t]*#\[cfg\(test\)\]\s*\n[ \t]*(?:pub(?:\([^)]*\))?\s+)?mod\s+[A-Za-z_0-9]+\s*\{" +) + def block_bodies(src: str, pattern: re.Pattern[str]) -> list[str]: """Bodies of every macro block `pattern` starts, brace-matched.""" @@ -97,10 +126,86 @@ def block_bodies(src: str, pattern: re.Pattern[str]) -> list[str]: return bodies +def brace_span(src: str, start: int) -> tuple[int, int]: + """`(open, close)` offsets of the brace-matched block opening at/after `start`.""" + i = src.index("{", start) + depth = 0 + j = i + while j < len(src): + if src[j] == "{": + depth += 1 + elif src[j] == "}": + depth -= 1 + if depth == 0: + break + j += 1 + return i, j + + +def cfg_test_module_files(root: Path, crates: list[str]) -> set[str]: + """Files whose entire module cannot exist in a shipping build. + + Seeded from every `#[cfg(test)] mod ;`, then closed transitively: a + plain `mod y;` inside an already-test-only file is test-only too, which is + what makes `gc/tests/mod.rs` carry its whole subtree. A `mod x;` in `a/b.rs` + (or `a/b/mod.rs`) resolves to `a/b/x.rs` or `a/b/x/mod.rs`; both spellings + are recorded, and a miss is simply a file that stays in scope. + """ + declares: dict[str, list[tuple[str, bool]]] = {} + for crate in crates: + base = root / crate + for dirpath, _dirs, files in os.walk(base): + for name in sorted(files): + if not name.endswith(".rs"): + continue + path = Path(dirpath) / name + rel = str(path.relative_to(root)) + src = path.read_text() + parent = path.parent if name in ("lib.rs", "mod.rs") else path.with_suffix("") + gated = set(CFG_TEST_MOD_RE.findall(src)) + edges = [] + for stem in set(ANY_MOD_RE.findall(src)): + for candidate in (parent / f"{stem}.rs", parent / stem / "mod.rs"): + if candidate.exists(): + edges.append((str(candidate.relative_to(root)), stem in gated)) + declares[rel] = edges + + test_only = {child for edges in declares.values() for child, gated in edges if gated} + changed = True + while changed: + changed = False + for rel in test_only & declares.keys(): + for child, _gated in declares[rel]: + if child not in test_only: + test_only.add(child) + changed = True + return test_only + + +def shipping_raw_blocks(src: str) -> int: + """Raw `thread_local!` blocks that survive into a non-test build. + + Skips a block carrying `#[cfg(test)]` directly above it, and any block + inside an inline `#[cfg(test)] mod … { … }`. + """ + gated_spans = [brace_span(src, m.start()) for m in CFG_TEST_INLINE_MOD_RE.finditer(src)] + count = 0 + for m in RAW_RE.finditer(src): + if any(open_at < m.start() < close_at for open_at, close_at in gated_spans): + continue + preceding = src[: m.start()].rstrip() + line = preceding[preceding.rfind("\n") + 1 :].strip() + if line == "#[cfg(test)]": + continue + count += 1 + return count + + def scan(root: Path, crates: list[str]) -> tuple[dict[str, int], int]: """Raw `thread_local!` blocks per file, and total hot declarations.""" raw: dict[str, int] = {} hot_declarations = 0 + test_only_files = cfg_test_module_files(root, crates) for crate in crates: base = root / crate for dirpath, _dirs, files in os.walk(base): @@ -113,9 +218,9 @@ def scan(root: Path, crates: list[str]) -> tuple[dict[str, int], int]: hot_declarations += sum( len(DECL_RE.findall(body)) for body in block_bodies(src, HOT_RE) ) - if rel in EXCLUDED: + if rel in EXCLUDED or rel in test_only_files: continue - count = len(RAW_RE.findall(src)) + count = shipping_raw_blocks(src) if count: raw[rel] = count return raw, hot_declarations @@ -251,12 +356,48 @@ def self_test() -> int: ) if not any("HOT_SLOT_CAPACITY" in p for p in verify(root, CRATES, allowlist)): failures.append("exceeding HOT_SLOT_CAPACITY passed") + (src_dir / "hot.rs").write_text( + "crate::perry_thread_local! { static B: u8 = const { 0 }; }\n" + ) + + # 5. The three `#[cfg(test)]` shapes are out of scope — and, the half + # that can go wrong quietly, REMOVING the gate puts them back in it. + (src_dir / "cold.rs").write_text("thread_local! { static A: u8 = const { 0 }; }\n") + write_allowlist(root, CRATES, allowlist) + gated = { + "attribute": "#[cfg(test)]\nthread_local! { static G: u8 = const { 0 }; }\n", + "inline mod": ( + "#[cfg(test)]\nmod t {\n" + " thread_local! { static G: u8 = const { 0 }; }\n" + "}\n" + ), + } + for shape, body in gated.items(): + (src_dir / "gated.rs").write_text(body) + if verify(root, CRATES, allowlist): + failures.append(f"a `#[cfg(test)]` {shape} declaration was counted") + (src_dir / "gated.rs").write_text(body.replace("#[cfg(test)]\n", "")) + if not verify(root, CRATES, allowlist): + failures.append(f"an UNGATED {shape} declaration passed") + (src_dir / "gated.rs").unlink() + + # 6. A whole file declared `#[cfg(test)] mod ;` is out of scope, + # and drops back in when the parent stops gating it. + (src_dir / "probes.rs").write_text( + "thread_local! { static G: u8 = const { 0 }; }\n" + ) + (src_dir / "lib.rs").write_text("#[cfg(test)]\nmod probes;\n") + if verify(root, CRATES, allowlist): + failures.append("a `#[cfg(test)] mod ;` file was counted") + (src_dir / "lib.rs").write_text("mod probes;\n") + if not verify(root, CRATES, allowlist): + failures.append("an ungated `mod ;` file passed") for f in failures: print(f"SELF-TEST FAILED: {f}", file=sys.stderr) if failures: return 1 - print("self-test: the checker can fail in all four directions") + print("self-test: the checker can fail in all six directions") return 0 diff --git a/scripts/thread_local_cold_allowlist.json b/scripts/thread_local_cold_allowlist.json index afde09619b..de275ddcdd 100644 --- a/scripts/thread_local_cold_allowlist.json +++ b/scripts/thread_local_cold_allowlist.json @@ -1,11 +1,10 @@ { "_comment": "Files still declaring raw `thread_local!`. Every entry is a declaration that pays `_tlv_get_addr` on Darwin; the count is a ratchet, so adding one to an already-listed file fails too. New code should use `crate::perry_thread_local!` \u2014 see crates/perry-runtime/src/tls_hot.rs. Regenerate with scripts/check_thread_locals.py --update.", - "_hot_declarations": 157, + "_hot_declarations": 160, "files": { "crates/perry-runtime/src/agent.rs": 1, - "crates/perry-runtime/src/arena/block.rs": 3, + "crates/perry-runtime/src/arena/block.rs": 2, "crates/perry-runtime/src/arena/page_meta.rs": 2, - "crates/perry-runtime/src/arena/quarantine.rs": 1, "crates/perry-runtime/src/async_context.rs": 2, "crates/perry-runtime/src/async_hooks.rs": 3, "crates/perry-runtime/src/builtins/arithmetic.rs": 1, @@ -33,36 +32,29 @@ "crates/perry-runtime/src/fs/stream.rs": 1, "crates/perry-runtime/src/gc/barrier.rs": 2, "crates/perry-runtime/src/gc/barrier_arming.rs": 1, - "crates/perry-runtime/src/gc/cycle.rs": 2, + "crates/perry-runtime/src/gc/cycle.rs": 1, "crates/perry-runtime/src/gc/dirty_page_cache.rs": 1, "crates/perry-runtime/src/gc/fromspace_scan.rs": 1, - "crates/perry-runtime/src/gc/layout.rs": 2, + "crates/perry-runtime/src/gc/layout.rs": 1, "crates/perry-runtime/src/gc/layout_tables.rs": 1, "crates/perry-runtime/src/gc/malloc.rs": 2, - "crates/perry-runtime/src/gc/mod.rs": 3, + "crates/perry-runtime/src/gc/mod.rs": 2, "crates/perry-runtime/src/gc/old_free.rs": 1, - "crates/perry-runtime/src/gc/oldgen_defrag.rs": 1, - "crates/perry-runtime/src/gc/policy.rs": 10, - "crates/perry-runtime/src/gc/promote_in_place.rs": 2, + "crates/perry-runtime/src/gc/policy.rs": 6, + "crates/perry-runtime/src/gc/promote_in_place.rs": 1, "crates/perry-runtime/src/gc/roots/scan_mode.rs": 1, "crates/perry-runtime/src/gc/roots/shadow_stack.rs": 2, "crates/perry-runtime/src/gc/roots/temp_roots.rs": 1, "crates/perry-runtime/src/gc/scan_fallback.rs": 1, - "crates/perry-runtime/src/gc/shape_install.rs": 2, - "crates/perry-runtime/src/gc/telemetry.rs": 3, + "crates/perry-runtime/src/gc/shape_install.rs": 1, + "crates/perry-runtime/src/gc/telemetry.rs": 2, "crates/perry-runtime/src/gc/tenuring.rs": 1, - "crates/perry-runtime/src/gc/tests/copying.rs": 1, - "crates/perry-runtime/src/gc/tests/roots.rs": 1, - "crates/perry-runtime/src/gc/tests/runtime_roots.rs": 4, - "crates/perry-runtime/src/gc/tests/runtime_roots/transient_handles.rs": 1, - "crates/perry-runtime/src/gc/tests/support.rs": 1, "crates/perry-runtime/src/gc/trace.rs": 2, - "crates/perry-runtime/src/gc/zeal.rs": 3, "crates/perry-runtime/src/intl/number_format.rs": 1, "crates/perry-runtime/src/iter_result.rs": 1, "crates/perry-runtime/src/json/mod.rs": 1, "crates/perry-runtime/src/json/raw_json.rs": 1, - "crates/perry-runtime/src/json_tape.rs": 3, + "crates/perry-runtime/src/json_tape.rs": 2, "crates/perry-runtime/src/json_tape_store.rs": 1, "crates/perry-runtime/src/media_playback.rs": 1, "crates/perry-runtime/src/native_arena.rs": 1, @@ -78,13 +70,11 @@ "crates/perry-runtime/src/node_submodules/test.rs": 1, "crates/perry-runtime/src/node_submodules/test_once_unit_tests.rs": 1, "crates/perry-runtime/src/node_submodules/test_property.rs": 1, - "crates/perry-runtime/src/node_submodules/tests.rs": 1, "crates/perry-runtime/src/node_submodules/trace_events.rs": 1, "crates/perry-runtime/src/object/native_module/callable_exports.rs": 2, - "crates/perry-runtime/src/object/spill.rs": 2, + "crates/perry-runtime/src/object/spill.rs": 1, "crates/perry-runtime/src/os/os_process_emitter.rs": 1, "crates/perry-runtime/src/os_process_streams.rs": 1, - "crates/perry-runtime/src/per_test_global.rs": 1, "crates/perry-runtime/src/perf_hooks.rs": 3, "crates/perry-runtime/src/process.rs": 2, "crates/perry-runtime/src/process/env_misc.rs": 3, From 00f41da7e03b8a44b6fcd0b4eaaa7d6146d424d3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Tue, 11 Aug 2026 03:43:27 +0200 Subject: [PATCH 2/2] docs(changelog): add fragment for the #7814 thread-local gate fix --- changelog.d/7824-thread-local-policy-gate.md | 21 ++++++++++++++++++++ 1 file changed, 21 insertions(+) create mode 100644 changelog.d/7824-thread-local-policy-gate.md diff --git a/changelog.d/7824-thread-local-policy-gate.md b/changelog.d/7824-thread-local-policy-gate.md new file mode 100644 index 0000000000..fbaa7651ad --- /dev/null +++ b/changelog.d/7824-thread-local-policy-gate.md @@ -0,0 +1,21 @@ +**The thread-local policy gate is green again, and now measures what it claims to.** +`scripts/check_thread_locals.py` was failing on `main`, which blocked +`tls-budget`'s documented promotion to a required context — a gate that runs, +reports failure, and cannot block anything. Two of the eight raw `thread_local!` +blocks were real, and they were not the ones reported: `array/indexing.rs`, +`map.rs`, `set.rs` and `registry_latch_probes.rs` are all `#[cfg(test)]` and do +not exist in a shipping build, while `gc/schedule.rs`'s `SAFEPOINT_COUNTER` +(every safepoint) and `SCHEDULE_NEXT_CANDIDATE_BYTES` (every poll) were live and +paying `_tlv_get_addr` on Darwin. Those two now use +`crate::perry_thread_local!`. The scan no longer counts `#[cfg(test)]` +declarations at all — recording one as "cold" records the wrong fact, since it +is not cold but absent — covering the attribute directly above a block, an +inline `#[cfg(test)] mod`, and a whole file declared `#[cfg(test)] mod ;` +(closed transitively, so `gc/tests/mod.rs` carries its subtree). Because an +over-broad exclusion would make the gate pass *by seeing less*, `--self-test` +now checks each shape in both directions — gated is skipped, and removing the +gate makes the same declaration fail again — proving six rejection directions +instead of four. The allowlist regeneration is provably one-way (101 → 91 +entries, 157 → 129 blocks, nothing added and no count increased), which also +retires three already-stale entries: `gc/zeal.rs` from #7741's removal of +`PERRY_GC_ZEAL`, plus `arena/quarantine.rs` and `gc/oldgen_defrag.rs`. (#7814)