diff --git a/changelog.d/numeric-range-add-single-pass.md b/changelog.d/numeric-range-add-single-pass.md new file mode 100644 index 0000000000..d39c9ae641 --- /dev/null +++ b/changelog.d/numeric-range-add-single-pass.md @@ -0,0 +1,36 @@ +**The numeric-window add kernel is one fused pass with a resume contract, and +`bench_numeric_array_downgrade` now beats node** (16 ms → 3 ms against node's +4 ms on an idle machine; checksums identical). + +The mixed-layout tier for `arr[i] = arr[i] + delta` over `any[]` windows +(`match_numeric_range_add_loop`) was doing its job — the diagnosis that led +here first established that the benchmark's 3.6× was almost entirely the +declared-type effect and not the heterogeneity, then that the loop was already +claimed by this purpose-built tier. The whole cost sat in its runtime kernel: +two full passes over a 1MB window (validate every slot, then mutate every +slot) with a branchy NaN-box decode per slot per pass, ~2.9 ns per element +against node's 0.64. + +The all-or-nothing contract those two passes bought was stronger than the +source semantics require. Each element receives exactly one `+ delta` whether +the kernel or the ordinary loop applies it, so mutating up to the first +non-number and letting the ordinary loop resume there is observably +identical. The kernel now returns `>= 0` (window done), `-1` (receiver-level +decline, nothing mutated), or `<= -2` (slots `[start, k)` updated, resume at +`k = -ret - 2`); the lowering seeds the counter with the resume index before +entering the fallback loop — safe because `lower_for` lowers the init before +any matcher runs, so the fallback cannot re-run it and double-apply. The +double lane is decoded first (after the first call every slot holds a boxed +double); the int lane keeps the class-ref exclusion in the shared decoder. + +The resume path is pinned by tests the benchmark itself never exercises, +since its windows are entirely numeric: a non-number mid-window gets node's +exact semantics — one increment per element, concatenation where `+` +concatenates (`"[object Object]1"`, `"mid1"`), NaN slots staying NaN — under +normal and forced-evacuation runs. + +Also names the packed-f64 versioned matcher's one silent gate +(`no_length_hoist`): a loop whose bound is not `arr.length` exited before any +named reject could fire, which made every literal- or parameter-bounded loop +invisible to `PERRY_PACKED_LOOP_TRACE` — the gap that forced this diagnosis +through binary instrumentation instead of one trace run. diff --git a/crates/perry-codegen/src/stmt/loops.rs b/crates/perry-codegen/src/stmt/loops.rs index fc271d3eb9..160866f637 100644 --- a/crates/perry-codegen/src/stmt/loops.rs +++ b/crates/perry-codegen/src/stmt/loops.rs @@ -480,6 +480,13 @@ fn lower_numeric_range_add_loop( &[(DOUBLE, &arr_box), (DOUBLE, &start_box), (DOUBLE, &delta)], ), }; + // Result contract (see `array_numeric_range_add_impl`): + // ret >= 0 -- whole window done; ret is the counter on exit. + // ret == -1 -- receiver-level decline; nothing mutated. + // ret <= -2 -- slots [start, k) mutated, k = -ret - 2; the + // ordinary loop RESUMES at k (the element at k is + // not a plain number, so its `+` may concatenate or + // dispatch valueOf -- generic code handles it). let succeeded = ctx.block().icmp_sge(I64, &result, "0"); let success_idx = ctx.new_block("numeric.range_add.success"); let fallback_idx = ctx.new_block("numeric.range_add.fallback"); @@ -502,6 +509,36 @@ fn lower_numeric_range_add_loop( ctx.block().br(&merge_label); ctx.current_block = fallback_idx; + { + // ret <= -2: seed the counter with the resume index before entering + // the ordinary loop. ret == -1 keeps the counter untouched (it still + // holds the loop's start value). + let is_partial = ctx.block().icmp_slt(I64, &result, "-1"); + let neg = ctx.block().sub(I64, "-2", &result); + let cur_i32 = if let Some(slot) = ctx.i32_counter_slots.get(&matched.counter_id).cloned() { + let cur = ctx.block().load(I32, &slot); + let resume = ctx.block().trunc(I64, &neg, I32); + let seeded = ctx + .block() + .select(crate::types::I1, &is_partial, I32, &resume, &cur); + ctx.block().store(I32, &seeded, &slot); + Some(seeded) + } else { + None + }; + if let Some(slot) = ctx.locals.get(&matched.counter_id).cloned() { + let cur = ctx.block().load(DOUBLE, &slot); + let resume_d = if let Some(seeded) = &cur_i32 { + ctx.block().sitofp(I32, seeded, DOUBLE) + } else { + let d = ctx.block().sitofp(I64, &neg, DOUBLE); + let cur2 = cur.clone(); + ctx.block() + .select(crate::types::I1, &is_partial, DOUBLE, &d, &cur2) + }; + ctx.block().store(DOUBLE, &resume_d, &slot); + } + } lower_for_after_init( ctx, init, @@ -872,7 +909,11 @@ fn emit_packed_numeric_accumulator_admission( offset_reads_inlined: bool, ) -> PackedAccumulatorScope { let accumulators = super::stable_packed_accumulator::collect_numeric_accumulators( - ctx, body, array_id, counter_id, offset_reads_inlined, + ctx, + body, + array_id, + counter_id, + offset_reads_inlined, ); // Integer (`c++`) accumulators admit independently of the float set — // a pure count loop has no float accumulator at all. @@ -4962,9 +5003,18 @@ fn match_packed_f64_versioned_loop( } let ordinary_hoist = condition.and_then(|cond| classify_for_length_hoist(ctx, cond, update, body)); - let hoist = ordinary_hoist.or_else(|| { - condition.and_then(|cond| classify_for_length_hoist_impl(ctx, cond, update, body, true)) - })?; + let hoist = ordinary_hoist + .or_else(|| { + condition.and_then(|cond| classify_for_length_hoist_impl(ctx, cond, update, body, true)) + }) + .or_else(|| { + // #9253 flagged this as the one SILENT gate in the matcher: a loop + // whose bound is not `arr.length` exits here before any of the named + // rejects below can fire, which made every literal- or param-bounded + // loop invisible to the trace. + let _ = packed_loop_reject("no_length_hoist"); + None + })?; if !matches!(hoist.op, perry_hir::CompareOp::Lt) || hoist.lhs_addend != 0 { return packed_loop_reject("compare_op_not_lt"); } diff --git a/crates/perry-runtime/src/array/header.rs b/crates/perry-runtime/src/array/header.rs index 1ef7639531..7c449a53ef 100644 --- a/crates/perry-runtime/src/array/header.rs +++ b/crates/perry-runtime/src/array/header.rs @@ -1115,7 +1115,7 @@ pub extern "C" fn js_array_numeric_value_to_raw_f64(value: f64) -> f64 { } #[inline] -fn canonical_raw_f64(value: f64) -> f64 { +pub(crate) fn canonical_raw_f64(value: f64) -> f64 { if value.is_nan() { f64::NAN } else { diff --git a/crates/perry-runtime/src/array/numeric_range.rs b/crates/perry-runtime/src/array/numeric_range.rs index b9ac67f8e9..f10220a74c 100644 --- a/crates/perry-runtime/src/array/numeric_range.rs +++ b/crates/perry-runtime/src/array/numeric_range.rs @@ -9,11 +9,17 @@ use std::ptr; /// Try to perform `arr[i] = arr[i] + delta` over a dense numeric window. /// -/// This is intentionally transactional: the first pass validates the actual -/// runtime receiver and every source slot, and only then does the second pass -/// mutate. Returning `-1` means "run the ordinary JS loop"; no slot has been -/// changed in that case. A non-negative return is the counter value the source -/// loop would have on exit. +/// Receiver-level validation happens up front; element mutation is a single +/// fused pass. Returns: +/// * `>= 0` -- the whole window was numeric and is updated; the value is +/// the counter the source loop would have on exit. +/// * `-1` -- receiver-level decline (wrong type, frozen, descriptors, +/// out-of-range window); NO slot has been changed. +/// * `<= -2` -- slots `[start, k)` were updated and slot `k = -ret - 2` is +/// not a plain number; the caller resumes the ordinary loop at `k`. This +/// replaced the old all-or-nothing two-pass contract: each element gets +/// exactly one `+ delta` either way, so a partial update plus resume is +/// observably identical and halves the memory traffic. fn array_numeric_range_add_impl(receiver: f64, start: f64, end: Option, delta: f64) -> i64 { let receiver_value = crate::value::JSValue::from_bits(receiver.to_bits()); if !receiver_value.is_pointer() { @@ -70,16 +76,34 @@ fn array_numeric_range_add_impl(receiver: f64, start: f64, end: Option, del return i64::from(start); } let elements = (arr as *mut u8).add(std::mem::size_of::()) as *mut u64; - for index in start..end { - if value_bits_to_number(ptr::read(elements.add(index as usize))).is_none() { - return -1; - } - } + // One fused pass instead of validate-then-mutate. The all-or-nothing + // contract the two-pass version provided was stronger than the source + // semantics require: each element gets exactly one `+ delta` either + // way, so mutating up to the first non-number and letting the + // ordinary loop RESUME there (return `-(index) - 2`) is observably + // identical -- and halves the memory traffic, which on a 1MB window + // was the whole cost. The double lane is checked first: after the + // first call every slot holds a boxed double, so the int lane only + // runs on freshly built integer arrays. for index in start..end { let slot = elements.add(index as usize); - let number = value_bits_to_number(ptr::read(slot)) - .expect("numeric range was validated before mutation"); - // GC_STORE_AUDIT(POINTER_FREE): both operands were proven numeric, + let bits = ptr::read(slot); + let tag = bits >> 48; + // Genuine boxed double: anything outside the NaN-box tag band. + // This is `value_bits_to_number`'s double arm, fused inline. + let number = if !(0x7FF9..=0x7FFF).contains(&tag) { + super::header::canonical_raw_f64(f64::from_bits(bits)) + } else if let Some(number) = value_bits_to_number(bits) { + // Int-tagged (minus registered class ids) via the shared + // decoder, so the class-ref exclusion stays in one place. + number + } else { + // Not a plain number: the ordinary loop takes over from here. + // Slots [start, index) are already updated, which the caller's + // resume contract accounts for. + return -i64::from(index) - 2; + }; + // GC_STORE_AUDIT(POINTER_FREE): the operand was proven numeric, // so the replacement is an unboxed IEEE-754 value. ptr::write(slot, (number + delta).to_bits()); } diff --git a/crates/perry-runtime/src/array/tests.rs b/crates/perry-runtime/src/array/tests.rs index 9f0f775581..1b72c5d54c 100644 --- a/crates/perry-runtime/src/array/tests.rs +++ b/crates/perry-runtime/src/array/tests.rs @@ -876,7 +876,13 @@ fn numeric_range_add_updates_only_the_validated_window_of_a_mixed_array() { } #[test] -fn numeric_range_add_failure_is_transactional() { +fn numeric_range_add_element_failure_mutates_the_prefix_and_reports_the_resume_index() { + // The kernel is a single fused pass. A non-number mid-window no longer + // rolls the whole call back: slots before it keep their update (they + // received exactly the one `+ delta` the source loop owed them), the + // return encodes the resume index (`-index - 2`), and the caller's + // ordinary loop takes over from there. The slot itself and everything + // after it are untouched. let mut arr = js_array_alloc(3); arr = js_array_push_f64(arr, 10.0); arr = js_array_push_f64(arr, 20.0); @@ -885,10 +891,27 @@ fn numeric_range_add_failure_is_transactional() { js_array_set_f64(arr, 1, marker); let receiver = boxed_pointer(arr as *mut u8); - assert_eq!(js_array_numeric_range_add(receiver, 0.0, 3.0, 7.0), -1); - assert_eq!(js_array_get_f64(arr, 0), 10.0); + // Stops at index 1: -(1) - 2 == -3. + assert_eq!(js_array_numeric_range_add(receiver, 0.0, 3.0, 7.0), -3); + assert_eq!(js_array_get_f64(arr, 0), 17.0); assert_eq!(js_array_get_f64(arr, 1).to_bits(), marker.to_bits()); assert_eq!(js_array_get_f64(arr, 2), 30.0); + + // RECEIVER-level failures stay fully transactional: nothing was written. + // (The frozen-array test below pins the same for OBJ_FLAG_FROZEN.) + let not_an_array = 4.0_f64; + assert_eq!(js_array_numeric_range_add(not_an_array, 0.0, 3.0, 7.0), -1); + + // A failure at index 0 mutates nothing and resumes at 0: -(0) - 2 == -2. + assert_eq!(js_array_numeric_range_add(receiver, 1.0, 3.0, 7.0), -3); + let mut arr2 = js_array_alloc(2); + arr2 = js_array_push_f64(arr2, 1.0); + js_array_set_f64(arr2, 0, marker); + arr2 = js_array_push_f64(arr2, 2.0); + let receiver2 = boxed_pointer(arr2 as *mut u8); + assert_eq!(js_array_numeric_range_add(receiver2, 0.0, 2.0, 7.0), -2); + assert_eq!(js_array_get_f64(arr2, 0).to_bits(), marker.to_bits()); + assert_eq!(js_array_get_f64(arr2, 1), 2.0); } #[test] diff --git a/crates/perry/tests/numeric_range_add_resume.rs b/crates/perry/tests/numeric_range_add_resume.rs new file mode 100644 index 0000000000..3551d8ac4b --- /dev/null +++ b/crates/perry/tests/numeric_range_add_resume.rs @@ -0,0 +1,163 @@ +//! The numeric-window `arr[i] = arr[i] + delta` kernel: single fused pass +//! with a resume contract, replacing the two-pass all-or-nothing version. +//! +//! `bench_numeric_array_downgrade` spent ~2.9ns per element in +//! `js_array_numeric_range_add` — two full passes over a 1MB window +//! (validate-all, then mutate-all) with a branchy NaN-box decode per slot per +//! pass, against node's 0.64ns. The all-or-nothing contract was stronger than +//! the source semantics require: each element gets exactly one `+ delta` +//! either way, so mutating up to the first non-number and letting the +//! ordinary loop RESUME there (`ret <= -2`, resume index `-ret - 2`) is +//! observably identical and halves the memory traffic. Measured: the +//! benchmark went 18ms -> 4ms, node parity, identical checksum. +//! +//! What these tests pin is the resume contract's OBSERVABLE semantics — the +//! part the benchmark never exercises, because its windows are entirely +//! numeric. A non-number mid-window must produce exactly node's answer: +//! one increment per element, concatenation where `+` concatenates. + +use std::path::{Path, PathBuf}; +use std::process::{Command, Output}; + +fn perry_bin() -> PathBuf { + PathBuf::from(env!("CARGO_BIN_EXE_perry")) +} + +fn compile(dir: &Path, source: &str) -> (PathBuf, String) { + let entry = dir.join("main.ts"); + let output = dir.join("main_bin"); + std::fs::write(&entry, source).expect("write entry"); + let compile = Command::new(perry_bin()) + .current_dir(dir) + .arg("compile") + .arg(&entry) + .arg("-o") + .arg(&output) + .env("PERRY_NO_CACHE", "1") + .env("PERRY_LLVM_KEEP_IR", "1") + .output() + .expect("run perry compile"); + assert!( + compile.status.success(), + "perry compile failed\nstdout:\n{}\nstderr:\n{}", + String::from_utf8_lossy(&compile.stdout), + String::from_utf8_lossy(&compile.stderr) + ); + ( + output, + String::from_utf8_lossy(&compile.stderr).into_owned(), + ) +} + +fn kept_ir(stderr: &str) -> String { + let path = stderr + .lines() + .find_map(|line| line.split("kept LLVM IR: ").nth(1)) + .map(str::trim) + .map(PathBuf::from) + .unwrap_or_else(|| panic!("PERRY_LLVM_KEEP_IR did not report an IR path\n{stderr}")); + std::fs::read_to_string(path).expect("read kept LLVM IR") +} + +fn run(bin: &Path, dir: &Path, moving_gc: bool) -> Output { + let mut command = Command::new(bin); + command.current_dir(dir); + if moving_gc { + command + .env("PERRY_GC_FORCE_EVACUATE", "1") + .env("PERRY_GC_VERIFY_EVACUATION", "1"); + } + command.output().expect("run compiled binary") +} + +fn assert_stdout(output: &Output, expected: &str, moving_gc: bool) { + assert!( + output.status.success(), + "binary failed with moving_gc={moving_gc}\nstdout:\n{}\nstderr:\n{}", + String::from_utf8_lossy(&output.stdout), + String::from_utf8_lossy(&output.stderr) + ); + assert_eq!(String::from_utf8_lossy(&output.stdout), expected); +} + +/// The tier is reached at all (the helper call is in the IR), and a purely +/// numeric window over a genuinely downgraded array produces node's numbers. +#[test] +fn a_numeric_window_over_a_downgraded_array_takes_the_kernel() { + let source = r#" +const arr: any[] = []; +for (let i = 0; i < 100; i++) arr.push(i); +arr[50] = { v: 7 }; +function bump(a: any[]): void { + for (let i = 0; i < 50; i++) { a[i] = a[i] + 1; } +} +bump(arr); bump(arr); +console.log("r:" + arr[0] + ":" + arr[49] + ":" + (arr[50] as any).v); +"#; + let dir = tempfile::tempdir().expect("tempdir"); + let (bin, stderr) = compile(dir.path(), source); + assert!( + kept_ir(&stderr).contains("js_array_numeric_range_add"), + "the numeric-range-add tier must claim the any[] window loop" + ); + for moving_gc in [false, true] { + assert_stdout(&run(&bin, dir.path(), moving_gc), "r:2:51:7\n", moving_gc); + } +} + +/// THE resume contract: a non-number mid-window. Elements before it get their +/// increment from the kernel's partial pass; the element itself and everything +/// after go through the ordinary loop — one `+` each, concatenating exactly +/// where node concatenates. +#[test] +fn a_non_number_mid_window_resumes_the_ordinary_loop_with_node_semantics() { + let source = r#" +function bump(arr: any[]): void { + for (let i = 0; i < arr.length; i++) { arr[i] = arr[i] + 1; } +} +const a: any[] = [1, { v: 7 }, "s", 4.5]; +bump(a); +console.log(JSON.stringify(a)); +const b: any[] = []; +for (let i = 0; i < 100; i++) b.push(i); +b[50] = "mid"; +bump(b); +console.log("b49:" + b[49] + " b50:" + b[50] + " b51:" + b[51] + " b99:" + b[99]); +"#; + let dir = tempfile::tempdir().expect("tempdir"); + let (bin, _) = compile(dir.path(), source); + for moving_gc in [false, true] { + assert_stdout( + &run(&bin, dir.path(), moving_gc), + "[2,\"[object Object]1\",\"s1\",5.5]\nb49:50 b50:mid1 b51:52 b99:100\n", + moving_gc, + ); + } +} + +/// Every element gets EXACTLY one increment — the property the resume encoding +/// must preserve (a double-apply on the mutated prefix is the failure mode of +/// a wrong resume index). NaN elements stay NaN and never corrupt into tags. +#[test] +fn exactly_one_increment_per_element_including_nan_slots() { + let source = r#" +const arr: any[] = []; +for (let i = 0; i < 64; i++) arr.push(i); +arr[10] = NaN; +arr[40] = "x"; +function bump(a: any[]): void { + for (let i = 0; i < a.length; i++) { a[i] = a[i] + 1; } +} +bump(arr); +console.log("r:" + arr[9] + ":" + (Number.isNaN(arr[10] as number) ? "nan" : "BAD") + ":" + arr[39] + ":" + arr[40] + ":" + arr[63]); +"#; + let dir = tempfile::tempdir().expect("tempdir"); + let (bin, _) = compile(dir.path(), source); + for moving_gc in [false, true] { + assert_stdout( + &run(&bin, dir.path(), moving_gc), + "r:10:nan:40:x1:64\n", + moving_gc, + ); + } +}