perf(regex): construction skips the write barrier's parent classification

changelog.d/9885-regex-newborn-barrier-gate.md

@@ -0,0 +1,43 @@
+**A `RegExp` literal's construction no longer pays the write barrier's parent
+classification.** Since #9845 the `RegExpHeader` is a nursery allocation, so
+its two string field stores — `pattern_ptr` and `flags_ptr` — cannot owe the
+remembered set anything; they were still taking the full barrier and
+discovering that fact, twice, at a cost of four page-map classifications, two
+dirty-page-cache probes and two child classifications per construction, all
+ending at `ParentNotOldSkips`.
+
+The fix is the runtime twin of a gate the compiler already emits in front of
+every one of its own stores (`emit_parent_may_need_remembering_check`, #7511):
+`GC_FLAG_TENURED` clear on the parent's live header **and** a globally idle
+incremental mark barrier ⇒ neither the remembered set nor the SATB shading has
+anything to record. Both clauses are read live, so a header a collection
+promoted between `arena_alloc_gc` and the store, or a
+`RegExp.prototype.compile` reassigning a tenured receiver, still takes the
+full path.
+
+Why the two clauses and not one: the tenured bit answers the generational
+question, and the incremental count is what makes it legal to skip the
+insertion shading as well — dropping either is a live child swept, which is
+what `gc::tests::inline_generation_gate_contract` already pins for the emitted
+gate and now pins for the runtime twin, clause by clause, against the same
+codegen predicate. A third test asserts on the header `js_regexp_new` actually
+returns, so the skip arm is proven reached rather than merely available.
+
+Measured motivation (segment-loop probe, region B, 60,000 reps, `sample`, main
+thread, leaf sum = thread header exactly): the probe constructs one `RegExp`
+per grapheme from a literal inside a function body, and the barrier subtree
+under `js_regexp_new` is 739 of 14,628 main-thread samples — 32 % of that
+function's own subtree.
+
+`PERRY_REGEX_NEWBORN_BARRIER_GATE=0` restores the unconditional pair; with the
+gate off nothing else changes, so the OFF arm is the pre-change code path
+exactly rather than a handicapped control.
+
+`PERRY_REGEX_DIAG` gains the counters that make the claim checkable rather
+than argued: `barrier_taken` / `barrier_gated` (whose sum must equal `new`),
+`header_bytes`, `site_verify_bytes` (the site cache's byte-compare volume,
+which `pattern_bytes` does not isolate) and `side_table_inserts`. Two
+reliability fixes ride along: a diag file the process cannot write now says so
+on stderr and falls back there instead of vanishing silently, and the first
+snapshot is written at the first tick rather than after a full second, so a
+short run can no longer look like a dead instrument.

crates/perry-runtime/src/gc/barrier_store.rs

@@ -319,3 +319,60 @@ pub(super) fn barrier_remembering_active() -> bool {
     bump_write_barrier_trace_counter(BarrierTraceCounter::UnarmedSkips);
     false
 }
+
+/// The runtime twin of codegen's `emit_parent_may_need_remembering_check`
+/// (#7511): may a store into a **freshly allocated** GC parent be skipped
+/// outright?
+///
+/// # Why this exists on the runtime side too
+///
+/// Every store emitted by the compiler is already gated this way — the
+/// generated code reads the parent's `gc_flags` and, when `GC_FLAG_TENURED`
+/// is clear *and* no incremental cycle is live anywhere, jumps over the
+/// barrier call entirely. Runtime-Rust construction paths call
+/// [`runtime_write_barrier_gc_slot`] unconditionally instead, so a native
+/// header born in the nursery pays, per pointer slot: a page-map
+/// classification for the malloc-parent probe, `barrier_child_prologue`, the
+/// armed check, the dereferenceability test, the dirty-page-cache probe and a
+/// second page-map classification in `barrier_parent_needs_remembering` —
+/// all of which end at `ParentNotOldSkips` because the parent is young.
+///
+/// Measured on the segment-loop probe (region B, 60,000 reps, `sample`, main
+/// thread): `js_regexp_new` constructs one `RegExp` per grapheme and its
+/// barrier subtree — `runtime_write_barrier_gc_slot` /
+/// `write_barrier_slot_decoded` / `write_barrier_decoded_parent` /
+/// `mark_dirty_external_slot_page` / `remembered_child_needs_tracking` /
+/// `classify_heap_generation_uncached` — is 739 of the 14,628 main-thread
+/// samples, 32 % of that function's own subtree.
+///
+/// # Why it is sound
+///
+/// Exactly the two clauses the emitted gate uses, for exactly the two reasons
+/// its doc comment gives:
+///
+///   * **`GC_FLAG_TENURED` clear** ⇒ the parent is not in the old generation,
+///     so no old→young remembered-set entry can be owed. The flag is read
+///     LIVE at the store, not claimed statically, because promotion can move
+///     an object under any static proof (#7501) — a header that a collection
+///     promoted between its allocation and this store reads TENURED here and
+///     takes the full barrier.
+///   * **`PERRY_INCREMENTAL_MARK_BARRIER_ACTIVE_COUNT == 0`** ⇒ no thread has
+///     an incremental mark barrier installed, which is what makes it legal to
+///     skip the SATB/insertion shading as well
+///     ([`incremental_mark_barrier_globally_idle`]). Shading is not a
+///     generational question and must never be dropped while a cycle is live,
+///     so a non-zero count sends the store down the ordinary path.
+///
+/// # Safety
+///
+/// Dereferences `parent_addr - GC_HEADER_SIZE`. The caller must pass a live,
+/// non-forwarded GC user pointer it has just allocated (or otherwise
+/// validated) — the same contract `emit_parent_may_need_remembering_check`
+/// places on its caller.
+#[inline]
+pub(crate) unsafe fn newborn_parent_needs_barrier(parent_addr: usize) -> bool {
+    if !super::barrier::incremental_mark_barrier_globally_idle() {
+        return true;
+    }
+    (*super::layout::header_from_user_ptr(parent_addr as *const u8)).gc_flags & GC_FLAG_TENURED != 0
+}

crates/perry-runtime/src/gc/tests/inline_generation_gate_contract.rs

@@ -262,3 +262,141 @@ fn the_incremental_clause_forces_the_call_for_a_young_parent() {
          store skips its insertion barrier and a live object is swept"
     );
 }
+
+// ---------------------------------------------------------------------------
+// The RUNTIME twin of the same gate.
+//
+// Runtime-Rust construction paths (`js_regexp_new` and friends) call the
+// barrier unconditionally, so a native header born in the nursery pays the
+// full parent classification on every field it initialises while generated
+// code, storing into the very same kind of object, skips it. These tests pin
+// `gc::newborn_parent_needs_barrier` to the emitted predicate CLAUSE FOR
+// CLAUSE, so the two can only drift by failing here.
+// ---------------------------------------------------------------------------
+
+/// Clause 1 (`GC_FLAG_TENURED`): the runtime twin must answer exactly what the
+/// emitted gate answers for the same live header.
+///
+/// Sabotage that this catches: a twin that only consults the incremental
+/// count answers "skip" for the tenured parent and fails the second assert —
+/// which is the stranded-child bug of
+/// `sabotaged_parent_gate_strands_a_young_child_the_shipped_gate_keeps`,
+/// reached from the runtime side instead of the emitted side.
+#[test]
+fn the_runtime_twin_reads_the_tenured_clause_from_the_live_header() {
+    let _guard = GcTestIsolationGuard::new();
+    assert!(
+        crate::gc::incremental_mark_barrier_globally_idle(),
+        "this test isolates clause 1, so no cycle may be live"
+    );
+
+    let young = crate::arena::arena_alloc_gc(64, 8, GC_TYPE_OBJECT) as usize;
+    assert_eq!(
+        header_flags(young) & GC_FLAG_TENURED,
+        0,
+        "a fresh nursery allocation must not be TENURED — otherwise this test exercises nothing"
+    );
+    assert!(
+        !unsafe { crate::gc::newborn_parent_needs_barrier(young) },
+        "a nursery parent with no cycle live is exactly the case the gate exists to skip"
+    );
+    assert_eq!(
+        unsafe { crate::gc::newborn_parent_needs_barrier(young) },
+        codegen_parent_may_need_remembering(header_flags(young), 0),
+        "the runtime twin and the emitted gate must agree on a nursery parent"
+    );
+
+    // The SAME address, now carrying the bit a promotion would have stamped.
+    // Read live, so this models a header a collection promoted between its
+    // allocation and the store that follows it.
+    unsafe { (*header_from_user_ptr(young as *const u8)).gc_flags |= GC_FLAG_TENURED };
+    assert!(
+        unsafe { crate::gc::newborn_parent_needs_barrier(young) },
+        "a TENURED parent owes the remembered set an entry and must take the full barrier"
+    );
+    assert_eq!(
+        unsafe { crate::gc::newborn_parent_needs_barrier(young) },
+        codegen_parent_may_need_remembering(header_flags(young), 0),
+        "the runtime twin and the emitted gate must agree on a tenured parent"
+    );
+    unsafe { (*header_from_user_ptr(young as *const u8)).gc_flags &= !GC_FLAG_TENURED };
+}
+
+/// Clause 2 (the incremental count): with a cycle live anywhere, a nursery
+/// parent must still take the call, because the skipped work includes the
+/// SATB/insertion shading and that is not a generational question.
+///
+/// Sabotage that this catches: a twin that only reads the header's flags
+/// answers "skip" while a cycle is marking, and a child linked in during that
+/// window is never shaded.
+#[test]
+fn the_runtime_twin_forces_the_barrier_while_an_incremental_cycle_is_live() {
+    let _guard = GcTestIsolationGuard::new();
+
+    let young = crate::arena::arena_alloc_gc(64, 8, GC_TYPE_OBJECT) as usize;
+    assert!(
+        !unsafe { crate::gc::newborn_parent_needs_barrier(young) },
+        "precondition: with no cycle live this parent is skipped"
+    );
+
+    crate::gc::PERRY_INCREMENTAL_MARK_BARRIER_ACTIVE_COUNT.fetch_add(1, Ordering::Relaxed);
+    let forced = unsafe { crate::gc::newborn_parent_needs_barrier(young) };
+    let codegen_answer = codegen_parent_may_need_remembering(
+        header_flags(young),
+        crate::gc::PERRY_INCREMENTAL_MARK_BARRIER_ACTIVE_COUNT.load(Ordering::Relaxed),
+    );
+    crate::gc::PERRY_INCREMENTAL_MARK_BARRIER_ACTIVE_COUNT.fetch_sub(1, Ordering::Relaxed);
+
+    assert!(
+        forced,
+        "a live incremental cycle must force the call even for a nursery parent — dropping this \
+         clause skips the insertion shading and sweeps a live child"
+    );
+    assert_eq!(
+        forced, codegen_answer,
+        "the runtime twin and the emitted gate must agree while a cycle is live"
+    );
+    assert!(
+        !unsafe { crate::gc::newborn_parent_needs_barrier(young) },
+        "the count is back to zero, so the same parent is skippable again"
+    );
+}
+
+/// **Did this code run?** The gate is only worth anything if the real
+/// `js_regexp_new` header reaches its skip arm — a fast path that is available
+/// but never taken is the campaign's "measured flat" failure in advance.
+///
+/// Asserts on the header `js_regexp_new` actually produced, not on a synthetic
+/// fixture: since #9845 it is a nursery allocation, so with no cycle live the
+/// two `pattern_ptr` / `flags_ptr` stores skip the barrier entirely, and the
+/// SAME header answers "take the barrier" the moment it carries the bit a
+/// promotion would stamp.
+#[cfg(feature = "regex-engine")]
+#[test]
+fn a_freshly_constructed_regexp_header_reaches_the_skip_arm() {
+    let _guard = GcTestIsolationGuard::new();
+
+    let pattern = crate::string::js_string_from_bytes(b"a(b)c".as_ptr(), 5);
+    let flags = crate::string::js_string_from_bytes(b"g".as_ptr(), 1);
+    let re = crate::regex::js_regexp_new(pattern, flags) as usize;
+
+    assert_eq!(
+        crate::arena::classify_heap_generation(re),
+        crate::arena::HeapGeneration::Nursery,
+        "#9845 allocates the RegExp header in the NURSERY; if that changes, the construction path \
+         stops being the case this gate is written for"
+    );
+    assert!(
+        !unsafe { crate::gc::newborn_parent_needs_barrier(re) },
+        "the construction path must actually TAKE the skip arm — an available-but-unreached fast \
+         path is indistinguishable from no fast path in a measurement"
+    );
+
+    unsafe { (*header_from_user_ptr(re as *const u8)).gc_flags |= GC_FLAG_TENURED };
+    assert!(
+        unsafe { crate::gc::newborn_parent_needs_barrier(re) },
+        "the same header, promoted, must take the full barrier — this is the \
+         `RegExp.prototype.compile`-on-a-tenured-receiver case"
+    );
+    unsafe { (*header_from_user_ptr(re as *const u8)).gc_flags &= !GC_FLAG_TENURED };
+}

crates/perry-runtime/src/hot_diag.rs

@@ -37,13 +37,25 @@ fn sink_from_env(name: &str) -> Option<Sink> {
     }
 }
 
+/// A failed file write used to be swallowed (`if ... .is_ok()`), so an
+/// unwritable path — a directory that does not exist, a read-only mount, a
+/// sandbox — produced *no file and no message*, which greps identically to
+/// "this instrument was never built". That is the campaign's own
+/// missing-exit-line trap in a second form, and it cost a lane a measurement
+/// run. Report the first failure on stderr, naming the path and the error,
+/// and keep writing there.
 fn write_sink(sink: &Sink, text: &str) {
     match sink {
         Sink::Stderr => eprint!("{text}"),
         Sink::File(path) => {
             let tmp = format!("{path}.tmp");
-            if std::fs::write(&tmp, text).is_ok() {
-                let _ = std::fs::rename(&tmp, path);
+            let wrote = std::fs::write(&tmp, text).and_then(|()| std::fs::rename(&tmp, path));
+            if let Err(err) = wrote {
+                static WARNED: AtomicBool = AtomicBool::new(false);
+                if !WARNED.swap(true, Ordering::Relaxed) {
+                    eprintln!("[hot-diag] cannot write {path}: {err} — falling back to stderr");
+                }
+                eprint!("{text}");
             }
         }
     }
@@ -140,6 +152,28 @@ pub struct RegexDiag {
     /// meta edge was wired for RegExp this was 0 by construction: the filter
     /// answered "maybe" for every one of them.
     pub desc_regexp_meta_negative: u64,
+    /// Constructions whose two header string stores took the full write
+    /// barrier pair (`GC_FLAG_TENURED` set on the freshly allocated header,
+    /// or an incremental cycle live anywhere).
+    pub new_barrier_taken: u64,
+    /// Constructions the newborn-parent gate proved owe the remembered set
+    /// nothing, so neither barrier call ran. `taken + gated == new_calls`
+    /// is the invariant: a run where `gated` is 0 did not exercise the gate.
+    pub new_barrier_gated: u64,
+    /// Bytes of `RegExpHeader` allocated by `js_regexp_new`. Load-independent
+    /// and directly comparable with a probe's allocation-per-grapheme reading.
+    pub new_header_bytes: u64,
+    /// Bytes the literal-site cache byte-compared to VERIFY a fingerprint
+    /// match (`site_cache::entry_matches`). Distinct from `pattern_bytes`,
+    /// which counts every construction's pattern length whether the probe hit
+    /// or missed: this is the `memcmp` volume alone, which is what a 12 KB
+    /// emoji pattern makes expensive and a 60-byte one does not.
+    pub new_site_verify_bytes: u64,
+    /// Address-keyed side-table inserts performed per construction
+    /// (`REGEX_POINTERS` and `REGEX_SOURCE_TABLE`) — two per header, each a
+    /// `PtrHasher` hash plus a hashbrown insert, mirrored by two removals at
+    /// death and two rekeys per evacuation.
+    pub new_side_table_inserts: u64,
     per_pattern: HashMap<usize, PatStat>,
 }
 
@@ -154,14 +188,17 @@ pub fn regex_with(f: impl FnOnce(&mut RegexDiag)) {
         let mut d = d.borrow_mut();
         if d.started.is_none() {
             d.started = Some(Instant::now());
-            d.last_dump = d.started;
+            // `last_dump` stays None so the FIRST tick dumps immediately: a
+            // run shorter than `DUMP_INTERVAL_MS` used to write nothing at
+            // all, which is indistinguishable from a dead instrument.
+            d.last_dump = None;
         }
         f(&mut d);
         d.events = d.events.wrapping_add(1);
         if d.events % TICK_EVERY == 0 {
             let due = d
                 .last_dump
-                .is_some_and(|t| t.elapsed().as_millis() >= DUMP_INTERVAL_MS);
+                .is_none_or(|t| t.elapsed().as_millis() >= DUMP_INTERVAL_MS);
             if due {
                 d.last_dump = Some(Instant::now());
                 if let Some(sink) = regex_sink() {
@@ -254,7 +291,9 @@ impl RegexDiag {
              compiles std={} fancy={} repeat={} cache_clears={} lazy_builds={} lazy_cache_hits={} \
              exec={} exec_matched={} capture_slots={} capture_bytes={} test={} test_global={} \
              match={} replace={} replace_matches={} split={} flags_alloc={} \
-             desc_regexp_probes={} desc_regexp_meta_negative={}",
+             desc_regexp_probes={} desc_regexp_meta_negative={} \
+             barrier_taken={} barrier_gated={} header_bytes={} site_verify_bytes={} \
+             side_table_inserts={}",
             self.new_calls,
             self.new_validated_hit,
             self.new_site_hit,
@@ -278,6 +317,11 @@ impl RegexDiag {
             self.new_flags_allocated,
             self.desc_regexp_probes,
             self.desc_regexp_meta_negative,
+            self.new_barrier_taken,
+            self.new_barrier_gated,
+            self.new_header_bytes,
+            self.new_site_verify_bytes,
+            self.new_side_table_inserts,
         );
         // Merge by content (prefix, len, flags): distinct literal sites with
         // the same pattern are one row.