Merge train: #9881, #9894, #9896

changelog.d/9879-gc-family-list-swap-remove.md

@@ -0,0 +1,29 @@
+### Fixed
+
+- **gc:** a keys-array family's descriptor list no longer memmoves its whole
+  tail on every removal, and no longer scans linearly to find an id.
+
+  `IdList::remove` was `Vec::remove(pos)`, which shifts everything past the
+  removed position. Measured on the compiled claude-code TUI, one 3300-char
+  reply, ten draws across two hosts: the removals sit at position **~0.31** of
+  the list — i.e. essentially always the front — and the longest list reaches
+  **514,030** entries, so the same ~3.7 M removals memmove up to **848 GB** in
+  a single turn. The removals come from the dead-owner prune
+  (`prune_dead_owner_side_tables_post_trace`).
+
+  No claim is made that this explains the turn's bimodal CPU: one draw moved
+  335 GB and was as fast as one that moved 16 GB, so bytes moved is necessary
+  but not sufficient for the slow mode. What is removed here is unambiguously
+  wasted work; how much time that is worth is for the A/B to say.
+
+  A spilled list now carries an `id -> index` map, built once it passes 32
+  entries, and `families` removes through a swap-remove that moves one element
+  regardless of position. `by_facts` keeps the order-preserving removal it
+  needs (its first entry is the canonical answer for exact-facts interning) and
+  is unaffected — measured at max length **1**, so it never builds an index.
+
+  The same index also removes the linear membership scan in
+  `family_push_back`, previously **6.2 %** of main-thread leaf samples.
+
+  This does not address why one family reaches half a million descriptors,
+  which is a separate defect and a separate change.

changelog.d/9894-perf-hooks-validation.md

@@ -0,0 +1,3 @@
+Make `Performance` methods reject invalid receivers and preserve
+`ERR_ILLEGAL_CONSTRUCTOR` when histogram constructor values are invoked with
+`new`.

changelog.d/9896-constructor-lowering-artifacts.md

@@ -0,0 +1,2 @@
+Preserve declarations, inline-cache globals, raw globals, and module counters
+when constructor lowering exits through its no-callable-parent fallback.

crates/perry-codegen/src/codegen/method.rs

@@ -33,6 +33,48 @@ pub(super) use typed::{
     compile_typed_i32_method, compile_typed_string_method,
 };
 
+struct LoweredFnArtifacts {
+    ic_globals: Vec<String>,
+    typed_parse_rodata: Vec<String>,
+    ic_end: u32,
+    pending_declares: Vec<(String, LlvmType, Vec<LlvmType>)>,
+    buffer_alias_used: u32,
+    native_rep_records: Vec<crate::native_value::NativeRepRecord>,
+}
+
+/// Detach everything a function body accumulated before releasing its borrow
+/// of the module-owned LLVM function.
+fn take_lowered_fn_artifacts(ctx: &mut FnCtx<'_>) -> LoweredFnArtifacts {
+    LoweredFnArtifacts {
+        ic_globals: std::mem::take(&mut ctx.ic_globals),
+        typed_parse_rodata: std::mem::take(&mut ctx.typed_parse_rodata),
+        ic_end: ctx.ic_site_counter,
+        pending_declares: std::mem::take(&mut ctx.pending_declares),
+        buffer_alias_used: ctx.buffer_data_slots.len() as u32,
+        native_rep_records: std::mem::take(&mut ctx.native_rep_records),
+    }
+}
+
+/// Publish the module-level artifacts emitted while lowering one function.
+/// Every exit after body lowering must go through this path: the function IR
+/// already references these names even when constructor setup bails out early.
+fn publish_lowered_fn_artifacts(llmod: &mut LlModule, artifacts: LoweredFnArtifacts) {
+    llmod.ic_counter = artifacts.ic_end;
+    llmod.buffer_alias_counter += artifacts.buffer_alias_used;
+    llmod
+        .native_rep_records
+        .extend(artifacts.native_rep_records);
+    for (name, ret, params) in artifacts.pending_declares {
+        llmod.declare_function(&name, ret, &params);
+    }
+    for ic_name in artifacts.ic_globals {
+        llmod.add_raw_global(crate::expr::inline_cache_global_definition(&ic_name));
+    }
+    for raw in artifacts.typed_parse_rodata {
+        llmod.add_raw_global(raw);
+    }
+}
+
 /// Compile a class instance method as a top-level LLVM function with the
 /// signature `perry_method_<class>_<name>(this_box: double, args: double…)
 /// -> double`. The first parameter (`this`) is stored in a slot whose
@@ -924,9 +966,9 @@ pub(super) fn compile_method(
                                 ));
                                 ctx.block().ret(DOUBLE, &undef);
                             }
-                            let _ = std::mem::take(&mut ctx.ic_globals);
-                            let _ = std::mem::take(&mut ctx.typed_parse_rodata);
-                            let _ = std::mem::take(&mut ctx.pending_declares);
+                            let artifacts = take_lowered_fn_artifacts(&mut ctx);
+                            drop(ctx);
+                            publish_lowered_fn_artifacts(llmod, artifacts);
                             return Ok(());
                         }
                     } else if let Some(ctor) = ctx.imported_class_ctors.get(&pname_owned).cloned() {
@@ -1303,12 +1345,7 @@ pub(super) fn compile_method(
             ctx.block().ret(DOUBLE, &return_value);
         }
     }
-    let ic_globals = std::mem::take(&mut ctx.ic_globals);
-    let typed_parse_rodata = std::mem::take(&mut ctx.typed_parse_rodata);
-    let ic_end = ctx.ic_site_counter;
-    let pending = std::mem::take(&mut ctx.pending_declares);
-    let buffer_alias_used = ctx.buffer_data_slots.len() as u32;
-    let native_rep_records = std::mem::take(&mut ctx.native_rep_records);
+    let artifacts = take_lowered_fn_artifacts(&mut ctx);
     drop(ctx);
 
     // Under native roots, ordinary `force_inline` is intentionally only an
@@ -1329,18 +1366,7 @@ pub(super) fn compile_method(
             lowered.pre_statepoint_inline = true;
         }
     }
-    llmod.ic_counter = ic_end;
-    llmod.buffer_alias_counter += buffer_alias_used;
-    llmod.native_rep_records.extend(native_rep_records);
-    for (name, ret, params) in pending {
-        llmod.declare_function(&name, ret, &params);
-    }
-    for ic_name in &ic_globals {
-        llmod.add_raw_global(crate::expr::inline_cache_global_definition(ic_name));
-    }
-    for raw in &typed_parse_rodata {
-        llmod.add_raw_global(raw.clone());
-    }
+    publish_lowered_fn_artifacts(llmod, artifacts);
     // The Phase 5a and nonnegative-index clones are purely additive: the
     // public symbol (and its trampoline/forwarder, if any) belongs to the
     // primary invocation. Emitting it again here would define that symbol
@@ -1852,24 +1878,44 @@ pub(super) fn compile_static_method(
             ctx.block().ret(DOUBLE, &undef);
         }
     }
-    let ic_globals = std::mem::take(&mut ctx.ic_globals);
-    let typed_parse_rodata = std::mem::take(&mut ctx.typed_parse_rodata);
-    let ic_end = ctx.ic_site_counter;
-    let pending = std::mem::take(&mut ctx.pending_declares);
-    let buffer_alias_used = ctx.buffer_data_slots.len() as u32;
-    let native_rep_records = std::mem::take(&mut ctx.native_rep_records);
+    let artifacts = take_lowered_fn_artifacts(&mut ctx);
     drop(ctx);
-    llmod.ic_counter = ic_end;
-    llmod.buffer_alias_counter += buffer_alias_used;
-    llmod.native_rep_records.extend(native_rep_records);
-    for (name, ret, params) in pending {
-        llmod.declare_function(&name, ret, &params);
-    }
-    for ic_name in &ic_globals {
-        llmod.add_raw_global(crate::expr::inline_cache_global_definition(ic_name));
-    }
-    for raw in &typed_parse_rodata {
-        llmod.add_raw_global(raw.clone());
-    }
+    publish_lowered_fn_artifacts(llmod, artifacts);
     Ok(())
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn lowered_function_artifacts_are_published_as_one_unit() {
+        let mut llmod = LlModule::new(crate::codegen::default_target_triple());
+        llmod.ic_counter = 3;
+        llmod.buffer_alias_counter = 7;
+
+        publish_lowered_fn_artifacts(
+            &mut llmod,
+            LoweredFnArtifacts {
+                ic_globals: vec!["perry_ic_9890".to_string()],
+                typed_parse_rodata: vec![
+                    "@issue_9890_rodata = private constant i64 9890".to_string()
+                ],
+                ic_end: 11,
+                pending_declares: vec![("js_issue_9890".to_string(), DOUBLE, vec![I64])],
+                buffer_alias_used: 2,
+                native_rep_records: Vec::new(),
+            },
+        );
+
+        assert_eq!(llmod.ic_counter, 11);
+        assert_eq!(llmod.buffer_alias_counter, 9);
+        let ir = llmod.to_ir();
+        assert!(ir.contains("@perry_ic_9890 ="), "{ir}");
+        assert!(
+            ir.contains("@issue_9890_rodata = private constant i64 9890"),
+            "{ir}"
+        );
+        assert!(ir.contains("declare double @js_issue_9890(i64)"), "{ir}");
+    }
+}

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

@@ -1899,6 +1899,7 @@ pub(super) fn run_copied_minor_attempt(
     }
     crate::arena::alloc_sample::report("minor");
     super::diag_sites::report_primitive_dispatch("minor");
+    crate::object::shapes::id_list_report();
     report_forwarding_refusals("copying_minor");
     super::scanner_profile::report_and_reset("copying_minor");
     CopiedMinorAttempt::Done(Some(CopiedMinorFastPathOutcome {

crates/perry-runtime/src/object/class_registry/construct.rs

@@ -331,6 +331,11 @@ pub unsafe extern "C-unwind" fn js_new_function_construct(
                 return result;
             }
         }
+        if module == "perf_histogram"
+            && matches!(method.as_str(), "RecordableHistogram" | "ELDHistogram")
+        {
+            return crate::perf_hooks::js_perf_illegal_constructor();
+        }
         if module == "sqlite"
             && matches!(
                 method.as_str(),

crates/perry-runtime/src/object/native_module.rs

@@ -23,7 +23,7 @@ mod callable_export_check;
 mod callable_export_table;
 pub(crate) mod callable_exports;
 mod perf_instance_bind;
-pub(crate) use perf_instance_bind::instance_bound_perf_method;
+pub(crate) use perf_instance_bind::{instance_bound_perf_method, performance_namespace_method};
 mod constants;
 mod constants_tables;
 mod constructor_exports;
@@ -1188,6 +1188,12 @@ pub extern "C" fn js_native_module_bind_method(
         }
     }
 
+    if let Some(value) =
+        performance_namespace_method(&module_name, property_name, namespace.get_nanbox_f64())
+    {
+        return value;
+    }
+
     // Check for known constant properties first
     if let Some(val) = unsafe {
         get_native_module_constant(&module_name, property_name, namespace.get_nanbox_f64())
@@ -1827,6 +1833,9 @@ unsafe fn vt_get_own_field(
     if let Some(value) = super::field_get_set::native_module_own_field_by_key(obj, key) {
         return Some(value);
     }
+    if let Some(value) = performance_namespace_method(&module_name, property_name, nb_ptr) {
+        return Some(JSValue::from_bits(value.to_bits()));
+    }
     // #3687: node:cluster default-import EventEmitter methods on the
     // distinct `cluster.default` namespace (see original comment at the
     // pre-relocation site in field_get_set.rs history).

crates/perry-runtime/src/object/native_module/constructor_exports.rs

@@ -16,6 +16,13 @@ pub(crate) fn is_native_module_constructor_export(module: &str, property: &str)
     let module = normalize_native_module_alias(module);
     let property = canonical_native_callable_property(module, property);
 
+    // Histogram constructors are only reachable through an instance's
+    // `constructor` property.  They are callable-shaped internal exports, and
+    // their construct path deliberately throws ERR_ILLEGAL_CONSTRUCTOR.
+    if module == "perf_histogram" && matches!(property, "RecordableHistogram" | "ELDHistogram") {
+        return true;
+    }
+
     if !is_native_module_callable_export(module, property) {
         return false;
     }
@@ -207,4 +214,16 @@ mod tests {
             "WriteStream"
         ));
     }
+
+    #[test]
+    fn histogram_class_values_are_constructor_shaped() {
+        assert!(is_native_module_constructor_export(
+            "perf_histogram",
+            "RecordableHistogram"
+        ));
+        assert!(is_native_module_constructor_export(
+            "perf_histogram",
+            "ELDHistogram"
+        ));
+    }
 }

crates/perry-runtime/src/object/native_module/perf_instance_bind.rs

@@ -54,3 +54,18 @@ pub(crate) fn instance_bound_perf_method(
         name.len(),
     ))
 }
+
+/// Return the receiver-aware method installed on `Performance.prototype` for
+/// reads from the canonical `performance` singleton. The singleton shares the
+/// `perf_hooks` dispatch tag with the module namespace, so identity distinguishes
+/// these methods from ordinary native-module exports.
+pub(crate) fn performance_namespace_method(
+    module_name: &str,
+    property_name: &str,
+    receiver: f64,
+) -> Option<f64> {
+    if module_name != "perf_hooks" || !crate::perf_hooks::is_performance_namespace_value(receiver) {
+        return None;
+    }
+    crate::perf_hooks::performance_prototype_method_value(property_name)
+}

crates/perry-runtime/src/object/shapes.rs

@@ -264,6 +264,15 @@ struct ShapeTableInner {
 
 const SHAPE_YOUNG_LOG_NAME: &str = "shapes.families+indices";
 
+/// Re-export of the id-list operation counters' report, so the collector does
+/// not have to name a private sibling module. One `[gc-idlist]` line per
+/// copying minor under `PERRY_GC_DIAG=1`; `elems_moved` is the falsifier for
+/// the swap-remove change.
+#[inline]
+pub(crate) fn id_list_report() {
+    shapes_store::id_list_report();
+}
+
 impl ShapeTableInner {
     /// Rule 1 of `gc/young_log.rs`: log a keys address BEFORE a family or a
     /// slot index is published under it, when the keys array is not old.
@@ -305,7 +314,14 @@ impl ShapeTableInner {
         let Some(ids) = self.families.get_mut(&keys) else {
             return false;
         };
-        let removed = ids.remove(id);
+        // UNORDERED: a family's readers are set-valued (see `IdList`'s type
+        // doc), and the ordered removal was memmoving the whole tail of a list
+        // measured at up to 514,030 entries, from position ~0.31, 3.7 M times
+        // per 3300-char reply. The dominant caller is the dead-owner prune
+        // (`prune_dead_owner_side_tables_post_trace` ->
+        // `remove_descriptor_indexed_under`); `retire_owned_shape_siblings`
+        // never sees a family longer than 16.
+        let removed = ids.remove_unordered(id);
         if ids.is_empty() {
             self.families.remove(&keys);
         }
@@ -334,7 +350,11 @@ impl ShapeTableInner {
         let Some(ids) = self.by_facts.get_mut(&facts) else {
             return false;
         };
-        let removed = ids.remove(id);
+        // ORDERED, and it must stay ordered: `facts_push_front` is how an
+        // installed process-global id becomes the canonical answer ahead of an
+        // equivalent local one, and this list is read first-wins. Measured at
+        // max length 1 on cc, so the order costs nothing to keep.
+        let removed = ids.remove_ordered(id);
         if ids.is_empty() {
             self.by_facts.remove(&facts);
         }