diff --git a/src/fable-library-py/.gitignore b/src/fable-library-py/.gitignore index 9ebbd6c985..b1fa2b1a4c 100644 --- a/src/fable-library-py/.gitignore +++ b/src/fable-library-py/.gitignore @@ -1,2 +1,3 @@ # Ignore compiled shared object files *.so +.benchmarks/ diff --git a/src/fable-library-py/Cargo.toml b/src/fable-library-py/Cargo.toml index 350c1383e2..2901613ece 100644 --- a/src/fable-library-py/Cargo.toml +++ b/src/fable-library-py/Cargo.toml @@ -20,3 +20,12 @@ regex = "1.12.4" [features] # must be enabled when building with `cargo build`, maturin enables this automatically extension-module = ["pyo3/extension-module"] + +# Release profile tuned for the extension module. maturin builds with `--release`, +# so it honors these settings. opt-level defaults to 3 for release; do NOT set +# `panic = "abort"` — PyO3 relies on unwinding to convert boundary panics into a +# Python `PanicException` rather than aborting the interpreter process. +[profile.release] +lto = "fat" +codegen-units = 1 +strip = true diff --git a/src/fable-library-py/src/array.rs b/src/fable-library-py/src/array.rs index 26d8b1f824..75e08ee860 100644 --- a/src/fable-library-py/src/array.rs +++ b/src/fable-library-py/src/array.rs @@ -14,7 +14,6 @@ use pyo3::{ prelude::*, types::{PyAnyMethods, PyList}, }; -use std::sync::{Arc, Mutex}; #[pyclass(module = "fable", subclass, from_py_object)] #[derive(Clone, Debug)] @@ -237,8 +236,7 @@ impl FSharpArray { } // Fallback to PyObject storage if type extraction fails. - // This allows for generic or mixed-type arrays, at the cost of dynamic dispatch and locking. - // Arc> is used for thread safety and Python interop. + // This allows for generic or mixed-type arrays, at the cost of dynamic dispatch. let len = elements.len().unwrap_or(0); let mut vec = Vec::with_capacity(len); @@ -248,7 +246,7 @@ impl FSharpArray { })?; Ok(FSharpArray { - storage: NativeArray::PyObject(Arc::new(Mutex::new(vec))), + storage: NativeArray::PyObject(vec), }) } @@ -369,7 +367,7 @@ impl FSharpArray { } Ok(FSharpArray { - storage: NativeArray::PyObject(Arc::new(Mutex::new(vec))), + storage: NativeArray::PyObject(vec), }) } @@ -946,8 +944,7 @@ impl FSharpArray { NativeArray::Bool(vec) => { vec[idx as usize] = value.extract()?; } - NativeArray::PyObject(arc_mutex_vec) => { - let mut vec = arc_mutex_vec.lock().unwrap(); // Acquire the lock + NativeArray::PyObject(vec) => { vec[idx as usize] = value.clone().into(); } } @@ -1162,12 +1159,10 @@ impl FSharpArray { let mut results = NativeArray::new(&original_type, None); // No initial capacity needed for i in 0..len { - // Avoid cloning item_obj if possible, only clone for predicate call - let item_obj = self.get_item_at_index(i as isize, py)?; - let keep = predicate.call1((item_obj.clone_ref(py),))?.is_truthy()?; - - if keep { - // Push the original item (by index) into the results collector + // The predicate consumes the fetched item; the kept value is copied + // straight from storage by index, so no extra clone is needed. + let item = self.storage.get(py, i)?; + if predicate.call1((item,))?.is_truthy()? { results.push_from_storage(&self.storage, i, py); } } @@ -1274,12 +1269,12 @@ impl FSharpArray { if len == 0 { // Return an empty array return Ok(FSharpArray { - storage: NativeArray::PyObject(Arc::new(Mutex::new(vec![]))), + storage: NativeArray::PyObject(vec![]), }); } // Create an array of arrays (chunks) - let mut chunks = NativeArray::PyObject(Arc::new(Mutex::new(vec![]))); + let mut chunks = NativeArray::PyObject(vec![]); // Create each chunk for x in 0..len.div_ceil(chunk_size) { @@ -1651,6 +1646,15 @@ impl FSharpArray { } pub fn sum(&self, py: Python<'_>, adder: &Bound<'_, PyAny>) -> PyResult> { + // Fast path: numeric storage sums natively without per-element Python calls. + // This bypasses `adder` entirely, which is safe because Fable only ever + // injects the standard numeric adder for primitive-typed arrays (custom + // monoids reach the runtime through `sum_by`, not `sum`). If that invariant + // ever changes, this fast path must be gated accordingly. + if let Some(result) = self.storage.try_native_sum(py) { + return result; + } + let len = self.storage.len(); let mut acc = adder.call_method0("GetZero")?; @@ -1670,6 +1674,14 @@ impl FSharpArray { )); } + // Fast path: sum numeric storage natively, then delegate the final + // DivideByInt to the averager so division/truncation semantics match exactly. + if let Some(sum_res) = self.storage.try_native_sum(py) { + let acc = sum_res?; + let result = averager.call_method1("DivideByInt", (acc, len))?; + return Ok(result.into()); + } + let mut acc = averager.call_method0("GetZero")?; for i in 0..len { @@ -1931,7 +1943,7 @@ impl FSharpArray { let len = self.storage.len(); if len == 0 { return Ok(FSharpArray { - storage: NativeArray::PyObject(Arc::new(Mutex::new(vec![]))), + storage: NativeArray::PyObject(vec![]), }); } @@ -2852,6 +2864,14 @@ impl FSharpArray { value: &Bound<'_, PyAny>, eq: Option<&Bound<'_, PyAny>>, ) -> PyResult { + // Fast path: with no custom equality comparer, numeric/bool storage scans + // natively (falls back when `value` isn't representable as the element type). + if eq.is_none() { + if let Some(result) = self.storage.try_native_contains(value) { + return result; + } + } + let len = self.storage.len(); for i in 0..len { let item = self.get_item_at_index(i as isize, py)?; @@ -2877,6 +2897,14 @@ impl FSharpArray { // let max (xs: 'a[]) ([] comparer: IComparer<'a>) : 'a = pub fn max(&self, py: Python<'_>, comparer: &Bound<'_, PyAny>) -> PyResult> { + // Fast path: integer storage compares natively (floats fall back to the + // comparer path to preserve its NaN ordering; empty falls back to the + // canonical empty-array error in reduce_impl). Bypassing `comparer` is safe + // because Fable always injects the standard comparer here; a custom key + // comparer reaches the runtime through `max_by`, which is not fast-pathed. + if let Some(result) = self.storage.try_native_max(py) { + return result; + } reduce_impl(self, py, |acc, item, py| { let comparison = comparer.call_method1("Compare", (acc.clone_ref(py), item.clone_ref(py)))?; @@ -2886,6 +2914,10 @@ impl FSharpArray { } pub fn min(&self, py: Python<'_>, comparer: &Bound<'_, PyAny>) -> PyResult> { + // Fast path: integer storage compares natively (see `max`). + if let Some(result) = self.storage.try_native_min(py) { + return result; + } reduce_impl(self, py, |acc, item, py| { let comparison = comparer.call_method1("Compare", (acc.clone_ref(py), item.clone_ref(py)))?; @@ -2947,7 +2979,7 @@ impl FSharpArray { let mut results = NativeArray::new(&target_type, None); for i in 0..len { let item = self.get_item_at_index(i as isize, py)?; - let chosen = chooser.call1((item.clone_ref(py),))?; + let chosen = chooser.call1((item,))?; if !chosen.is_none() { results.push_value(&chosen, py)?; } @@ -3282,8 +3314,7 @@ impl FSharpArray { NativeArray::Float32(vec) => vec.swap(i, j), NativeArray::Float64(vec) => vec.swap(i, j), NativeArray::Bool(vec) => vec.swap(i, j), - NativeArray::PyObject(arc) => { - let mut vec = arc.lock().unwrap(); + NativeArray::PyObject(vec) => { vec.swap(i, j); } } diff --git a/src/fable-library-py/src/native_array.rs b/src/fable-library-py/src/native_array.rs index 4039cf60f4..6aa607844f 100644 --- a/src/fable-library-py/src/native_array.rs +++ b/src/fable-library-py/src/native_array.rs @@ -1,12 +1,12 @@ use crate::floats::{Float32, Float64}; use crate::ints::{Int16, Int32, Int64, Int8, UInt16, UInt32, UInt64, UInt8}; use pyo3::class::basic::CompareOp; +use pyo3::intern; use pyo3::prelude::*; use pyo3::types::PyAnyMethods; use pyo3::IntoPyObjectExt; use std::cell::RefCell; use std::cmp::Ordering; -use std::sync::{Arc, Mutex}; #[derive(Clone, Debug, PartialEq)] pub enum ArrayType { @@ -127,7 +127,17 @@ pub enum NativeArray { Float32(Vec), Float64(Vec), Bool(Vec), - PyObject(Arc>>>), + // Generic storage. No Mutex needed: all mutation goes through `&mut` receivers, + // which PyO3 borrow-checks per pyclass instance (atomic `try_borrow_mut`), so a + // racing conflict raises `AlreadyBorrowed` instead of tearing the Vec — on both + // GIL and no-GIL builds. This works only *because* the Arc is also gone: the + // borrow flag is per-instance, so the old `Arc` aliasing (two arrays, one + // Vec) needed the Mutex; with one owner per Vec, the borrow checker suffices. + // (Like .NET arrays, this is still not thread-safe for unsynchronized writes — + // the Mutex only gave memory safety, not logical safety.) + // + // Cloning deep-copies the element references (F# value semantics, no aliasing). + PyObject(Vec>), } impl Clone for NativeArray { @@ -144,9 +154,12 @@ impl Clone for NativeArray { NativeArray::Float32(vec) => NativeArray::Float32(vec.clone()), NativeArray::Float64(vec) => NativeArray::Float64(vec.clone()), NativeArray::Bool(vec) => NativeArray::Bool(vec.clone()), - NativeArray::PyObject(arc) => { - // Clone the Arc, which increments the reference count - NativeArray::PyObject(Arc::clone(arc)) + NativeArray::PyObject(vec) => { + // Deep-copy: increment the refcount of each element so the clone + // owns independent references (value semantics, no aliasing). + Python::attach(|py| { + NativeArray::PyObject(vec.iter().map(|o| o.clone_ref(py)).collect()) + }) } } } @@ -240,7 +253,7 @@ impl NativeArray { NativeArray::Float32(vec) => vec.len(), NativeArray::Float64(vec) => vec.len(), NativeArray::Bool(vec) => vec.len(), - NativeArray::PyObject(vec) => vec.lock().unwrap().len(), + NativeArray::PyObject(vec) => vec.len(), } } @@ -252,7 +265,7 @@ impl NativeArray { pub fn equals(&self, other: &NativeArray, py: Python<'_>) -> bool { // Helper for comparing PyObject with other types fn compare_pyobject_with pyo3::FromPyObject<'a, 'py>>( - py_vec: &std::sync::MutexGuard>>, + py_vec: &[Py], other_vec: &[T], py: Python<'_>, ) -> bool { @@ -277,8 +290,8 @@ impl NativeArray { (NativeArray::Float64(vec), NativeArray::Float64(other_vec)) => vec == other_vec, (NativeArray::Bool(vec), NativeArray::Bool(other_vec)) => vec == other_vec, (NativeArray::PyObject(vec), NativeArray::PyObject(other_vec)) => { - let xs = vec.lock().unwrap(); - let ys = other_vec.lock().unwrap(); + let xs = vec; + let ys = other_vec; if xs.len() != ys.len() { return false; } @@ -290,36 +303,36 @@ impl NativeArray { }) } (NativeArray::PyObject(vec), other) => { - let xs = vec.lock().unwrap(); + let xs = vec; match other { - NativeArray::Int8(vec) => compare_pyobject_with(&xs, vec, py), - NativeArray::UInt8(vec) => compare_pyobject_with(&xs, vec, py), - NativeArray::Int16(vec) => compare_pyobject_with(&xs, vec, py), - NativeArray::UInt16(vec) => compare_pyobject_with(&xs, vec, py), - NativeArray::Int32(vec) => compare_pyobject_with(&xs, vec, py), - NativeArray::UInt32(vec) => compare_pyobject_with(&xs, vec, py), - NativeArray::Int64(vec) => compare_pyobject_with(&xs, vec, py), - NativeArray::UInt64(vec) => compare_pyobject_with(&xs, vec, py), - NativeArray::Float32(vec) => compare_pyobject_with(&xs, vec, py), - NativeArray::Float64(vec) => compare_pyobject_with(&xs, vec, py), - NativeArray::Bool(vec) => compare_pyobject_with(&xs, vec, py), + NativeArray::Int8(vec) => compare_pyobject_with(xs, vec, py), + NativeArray::UInt8(vec) => compare_pyobject_with(xs, vec, py), + NativeArray::Int16(vec) => compare_pyobject_with(xs, vec, py), + NativeArray::UInt16(vec) => compare_pyobject_with(xs, vec, py), + NativeArray::Int32(vec) => compare_pyobject_with(xs, vec, py), + NativeArray::UInt32(vec) => compare_pyobject_with(xs, vec, py), + NativeArray::Int64(vec) => compare_pyobject_with(xs, vec, py), + NativeArray::UInt64(vec) => compare_pyobject_with(xs, vec, py), + NativeArray::Float32(vec) => compare_pyobject_with(xs, vec, py), + NativeArray::Float64(vec) => compare_pyobject_with(xs, vec, py), + NativeArray::Bool(vec) => compare_pyobject_with(xs, vec, py), _ => false, } } (other, NativeArray::PyObject(vec)) => { - let ys = vec.lock().unwrap(); + let ys = vec; match other { - NativeArray::Int8(vec) => compare_pyobject_with(&ys, vec, py), - NativeArray::UInt8(vec) => compare_pyobject_with(&ys, vec, py), - NativeArray::Int16(vec) => compare_pyobject_with(&ys, vec, py), - NativeArray::UInt16(vec) => compare_pyobject_with(&ys, vec, py), - NativeArray::Int32(vec) => compare_pyobject_with(&ys, vec, py), - NativeArray::UInt32(vec) => compare_pyobject_with(&ys, vec, py), - NativeArray::Int64(vec) => compare_pyobject_with(&ys, vec, py), - NativeArray::UInt64(vec) => compare_pyobject_with(&ys, vec, py), - NativeArray::Float32(vec) => compare_pyobject_with(&ys, vec, py), - NativeArray::Float64(vec) => compare_pyobject_with(&ys, vec, py), - NativeArray::Bool(vec) => compare_pyobject_with(&ys, vec, py), + NativeArray::Int8(vec) => compare_pyobject_with(ys, vec, py), + NativeArray::UInt8(vec) => compare_pyobject_with(ys, vec, py), + NativeArray::Int16(vec) => compare_pyobject_with(ys, vec, py), + NativeArray::UInt16(vec) => compare_pyobject_with(ys, vec, py), + NativeArray::Int32(vec) => compare_pyobject_with(ys, vec, py), + NativeArray::UInt32(vec) => compare_pyobject_with(ys, vec, py), + NativeArray::Int64(vec) => compare_pyobject_with(ys, vec, py), + NativeArray::UInt64(vec) => compare_pyobject_with(ys, vec, py), + NativeArray::Float32(vec) => compare_pyobject_with(ys, vec, py), + NativeArray::Float64(vec) => compare_pyobject_with(ys, vec, py), + NativeArray::Bool(vec) => compare_pyobject_with(ys, vec, py), _ => false, } } @@ -403,8 +416,6 @@ impl NativeArray { Ok(()) } (NativeArray::PyObject(src), NativeArray::PyObject(dst)) => { - let src = src.lock().unwrap(); - let mut dst = dst.lock().unwrap(); for i in 0..count { dst[target_index + i] = src[source_index + i].clone_ref(py); } @@ -436,9 +447,9 @@ impl NativeArray { ArrayType::Float32 => NativeArray::Float32(Vec::with_capacity(capacity.unwrap_or(0))), ArrayType::Float64 => NativeArray::Float64(Vec::with_capacity(capacity.unwrap_or(0))), ArrayType::Bool => NativeArray::Bool(Vec::with_capacity(capacity.unwrap_or(0))), - ArrayType::Generic => NativeArray::PyObject(Arc::new(Mutex::new(Vec::with_capacity( - capacity.unwrap_or(0), - )))), + ArrayType::Generic => { + NativeArray::PyObject(Vec::with_capacity(capacity.unwrap_or(0))) + } } } @@ -496,7 +507,7 @@ impl NativeArray { ArrayType::Float32 => NativeArray::Float32(Vec::new()), ArrayType::Float64 => NativeArray::Float64(Vec::new()), ArrayType::Bool => NativeArray::Bool(Vec::new()), - ArrayType::Generic => NativeArray::PyObject(Arc::new(Mutex::new(Vec::new()))), + ArrayType::Generic => NativeArray::PyObject(Vec::new()), } } @@ -528,9 +539,7 @@ impl NativeArray { (NativeArray::Float64(dst), NativeArray::Float64(src)) => dst.push(src[index]), (NativeArray::Bool(dst), NativeArray::Bool(src)) => dst.push(src[index]), (NativeArray::PyObject(dst), NativeArray::PyObject(src)) => { - let src_guard = src.lock().unwrap(); - let mut dst_guard = dst.lock().unwrap(); - dst_guard.push(src_guard[index].clone_ref(py)); + dst.push(src[index].clone_ref(py)); } _ => panic!("Cannot push between different array types"), } @@ -556,8 +565,7 @@ impl NativeArray { NativeArray::Float64(vec) => push_typed_value!(vec, value), NativeArray::Bool(vec) => push_typed_value!(vec, value), NativeArray::PyObject(vec) => { - let mut guard = vec.lock().unwrap(); - guard.push(value.clone().unbind()); + vec.push(value.clone().unbind()); } } Ok(()) @@ -597,10 +605,9 @@ impl NativeArray { } } NativeArray::PyObject(vec) => { - let mut guard = vec.lock().unwrap(); let py_value = value.into_py_any(value.py())?; for i in 0..count { - guard[target_index + i] = py_value.clone_ref(value.py()); + vec[target_index + i] = py_value.clone_ref(value.py()); } } } @@ -623,15 +630,12 @@ impl NativeArray { NativeArray::Float32(vec) => NativeArray::Float32(reverse_vec!(vec)), NativeArray::Float64(vec) => NativeArray::Float64(reverse_vec!(vec)), NativeArray::Bool(vec) => NativeArray::Bool(reverse_vec!(vec)), - NativeArray::PyObject(arc) => { - let mut new_vec = Vec::with_capacity(arc.lock().unwrap().len()); - { - let guard = arc.lock().unwrap(); - for i in (0..guard.len()).rev() { - new_vec.push(guard[i].clone_ref(py)); - } + NativeArray::PyObject(vec) => { + let mut new_vec = Vec::with_capacity(vec.len()); + for i in (0..vec.len()).rev() { + new_vec.push(vec[i].clone_ref(py)); } - NativeArray::PyObject(Arc::new(Mutex::new(new_vec))) + NativeArray::PyObject(new_vec) } } } @@ -653,8 +657,8 @@ impl NativeArray { NativeArray::Float32(vec) => simple_vec_operation!(vec, remove, index), NativeArray::Float64(vec) => simple_vec_operation!(vec, remove, index), NativeArray::Bool(vec) => simple_vec_operation!(vec, remove, index), - NativeArray::PyObject(arc_vec) => { - arc_vec.lock().unwrap().remove(index); + NativeArray::PyObject(vec) => { + vec.remove(index); } } } @@ -692,8 +696,7 @@ impl NativeArray { let bool_value: bool = value.extract()?; vec.insert(index, bool_value); } - NativeArray::PyObject(arc_mutex_vec) => { - let mut vec = arc_mutex_vec.lock().unwrap(); + NativeArray::PyObject(vec) => { vec.insert(index, value.clone().into()); } } @@ -717,9 +720,7 @@ impl NativeArray { NativeArray::Float64(vec) => simple_vec_operation!(vec, truncate, new_size), NativeArray::Bool(vec) => simple_vec_operation!(vec, truncate, new_size), NativeArray::PyObject(vec) => { - if let Ok(mut vec) = vec.lock() { - vec.truncate(new_size); - } + vec.truncate(new_size); } } } @@ -827,8 +828,7 @@ impl NativeArray { } NativeArray::PyObject(vec) => { let error: RefCell> = RefCell::new(None); - let mut guard = vec.lock().unwrap(); - guard.sort_by(|a, b| { + vec.sort_by(|a, b| { if error.borrow().is_some() { return Ordering::Equal; } @@ -899,7 +899,7 @@ impl NativeArray { let proj_a = projection.call1((py_a,))?; let proj_b = projection.call1((py_b,))?; // Call the Compare method on the IComparer object - let result = comparer.call_method1("Compare", (proj_a, proj_b))?; + let result = comparer.call_method1(intern!(py, "Compare"), (proj_a, proj_b))?; Ok(result.extract::()?.cmp(&0)) } @@ -941,12 +941,7 @@ impl NativeArray { NativeArray::Bool(vec) => sort_projection_with_error!(vec, Bool), NativeArray::PyObject(vec) => { let error: RefCell> = RefCell::new(None); - let mut new_vec = vec - .lock() - .unwrap() - .iter() - .map(|x| x.clone_ref(py)) - .collect::>(); + let mut new_vec = vec.iter().map(|x| x.clone_ref(py)).collect::>(); new_vec.sort_by(|a, b| { if error.borrow().is_some() { return Ordering::Equal; @@ -956,7 +951,7 @@ impl NativeArray { let py_b = b.bind(py); let proj_a = projection.call1((py_a,))?; let proj_b = projection.call1((py_b,))?; - let cmp_result = comparer.call_method1("Compare", (proj_a, proj_b))?; + let cmp_result = comparer.call_method1(intern!(py, "Compare"), (proj_a, proj_b))?; Ok(cmp_result.extract::()?.cmp(&0)) })(); match result { @@ -970,7 +965,7 @@ impl NativeArray { if let Some(e) = error.into_inner() { return Err(e); } - NativeArray::PyObject(Arc::new(Mutex::new(new_vec))) + NativeArray::PyObject(new_vec) } }; Ok(result) @@ -994,10 +989,153 @@ impl NativeArray { NativeArray::Float32(vec) => Ok(vec[index].into_py_any(py)?), NativeArray::Float64(vec) => Ok(vec[index].into_py_any(py)?), NativeArray::Bool(vec) => Ok(vec[index].into_py_any(py)?), - NativeArray::PyObject(vec) => { - let guard = vec.lock().unwrap(); - Ok(guard[index].clone_ref(py)) - } + NativeArray::PyObject(vec) => Ok(vec[index].clone_ref(py)), + } + } + + /// Native `sum` fast path for numeric storage. + /// + /// Returns `Some(result)` for the numeric variants, computing the sum in a + /// tight unboxed loop and boxing only the final value; returns `None` for + /// `Bool`/`Generic` storage so the caller falls back to the Python-callback + /// path. + /// + /// Semantics match the Python `IGenericAdder` path: integer types use + /// **wrapping** arithmetic at the element width and the result is boxed as the + /// corresponding wrapper type (`Int32`, …) — the same type `GetZero`/`Add` + /// would produce — while floats use IEEE addition. + pub fn try_native_sum(&self, py: Python<'_>) -> Option>> { + macro_rules! sum_int { + ($vec:expr, $w:ident, $t:ty) => {{ + let mut acc: $t = 0; + for &x in $vec.iter() { + acc = acc.wrapping_add(x); + } + Some($w(acc).into_py_any(py)) + }}; + } + macro_rules! sum_float { + ($vec:expr, $w:ident, $t:ty) => {{ + let mut acc: $t = 0.0; + for &x in $vec.iter() { + acc += x; + } + Some($w(acc).into_py_any(py)) + }}; + } + match self { + NativeArray::Int8(v) => sum_int!(v, Int8, i8), + NativeArray::UInt8(v) => sum_int!(v, UInt8, u8), + NativeArray::Int16(v) => sum_int!(v, Int16, i16), + NativeArray::UInt16(v) => sum_int!(v, UInt16, u16), + NativeArray::Int32(v) => sum_int!(v, Int32, i32), + NativeArray::UInt32(v) => sum_int!(v, UInt32, u32), + NativeArray::Int64(v) => sum_int!(v, Int64, i64), + NativeArray::UInt64(v) => sum_int!(v, UInt64, u64), + NativeArray::Float32(v) => sum_float!(v, Float32, f32), + NativeArray::Float64(v) => sum_float!(v, Float64, f64), + NativeArray::Bool(_) | NativeArray::PyObject(_) => None, + } + } + + /// Native `max` fast path for **integer** storage. + /// + /// Returns `Some(result)` for non-empty integer variants, boxing the largest + /// element as a plain Python int (matching `get`, i.e. the value the existing + /// `reduce_impl`/comparer path returns). Returns `None` for empty arrays (so + /// the caller's `reduce_impl` produces the canonical empty-array error) and for + /// float/bool/generic storage (floats fall back to the comparer path to + /// preserve its NaN ordering semantics). + pub fn try_native_max(&self, py: Python<'_>) -> Option>> { + macro_rules! max_int { + ($vec:expr) => {{ + if $vec.is_empty() { + None + } else { + let mut acc = $vec[0]; + for &x in &$vec[1..] { + if x > acc { + acc = x; + } + } + Some(acc.into_py_any(py)) + } + }}; + } + match self { + NativeArray::Int8(v) => max_int!(v), + NativeArray::UInt8(v) => max_int!(v), + NativeArray::Int16(v) => max_int!(v), + NativeArray::UInt16(v) => max_int!(v), + NativeArray::Int32(v) => max_int!(v), + NativeArray::UInt32(v) => max_int!(v), + NativeArray::Int64(v) => max_int!(v), + NativeArray::UInt64(v) => max_int!(v), + _ => None, + } + } + + /// Native `min` fast path for **integer** storage. See [`try_native_max`]. + pub fn try_native_min(&self, py: Python<'_>) -> Option>> { + macro_rules! min_int { + ($vec:expr) => {{ + if $vec.is_empty() { + None + } else { + let mut acc = $vec[0]; + for &x in &$vec[1..] { + if x < acc { + acc = x; + } + } + Some(acc.into_py_any(py)) + } + }}; + } + match self { + NativeArray::Int8(v) => min_int!(v), + NativeArray::UInt8(v) => min_int!(v), + NativeArray::Int16(v) => min_int!(v), + NativeArray::UInt16(v) => min_int!(v), + NativeArray::Int32(v) => min_int!(v), + NativeArray::UInt32(v) => min_int!(v), + NativeArray::Int64(v) => min_int!(v), + NativeArray::UInt64(v) => min_int!(v), + _ => None, + } + } + + /// Native `contains` fast path for numeric/bool storage. + /// + /// Only used when no custom equality comparer is supplied. Extracts `value` to + /// the element type and scans natively; if `value` cannot be represented as the + /// element type, returns `None` so the caller falls back to the general + /// `rich_compare` path (which preserves Python cross-type equality semantics). + pub fn try_native_contains( + &self, + value: &Bound<'_, PyAny>, + ) -> Option> { + macro_rules! contains_prim { + ($vec:expr, $t:ty) => {{ + match value.extract::<$t>() { + Ok(v) => Some(Ok($vec.iter().any(|&x| x == v))), + Err(_) => None, + } + }}; + } + match self { + NativeArray::Int8(v) => contains_prim!(v, i8), + NativeArray::UInt8(v) => contains_prim!(v, u8), + NativeArray::Int16(v) => contains_prim!(v, i16), + NativeArray::UInt16(v) => contains_prim!(v, u16), + NativeArray::Int32(v) => contains_prim!(v, i32), + NativeArray::UInt32(v) => contains_prim!(v, u32), + NativeArray::Int64(v) => contains_prim!(v, i64), + NativeArray::UInt64(v) => contains_prim!(v, u64), + NativeArray::Float32(v) => contains_prim!(v, f32), + NativeArray::Float64(v) => contains_prim!(v, f64), + NativeArray::Bool(v) => contains_prim!(v, bool), + NativeArray::PyObject(_) => None, } } } diff --git a/src/fable-library-py/src/util.rs b/src/fable-library-py/src/util.rs index 3992ef47db..0f782944dd 100644 --- a/src/fable-library-py/src/util.rs +++ b/src/fable-library-py/src/util.rs @@ -1,4 +1,5 @@ use pyo3::class::basic::CompareOp; +use pyo3::intern; use pyo3::{prelude::*, types::PyAnyMethods}; /// Helper class for sort_in_place_by that applies projection before comparison @@ -32,11 +33,13 @@ impl ProjectionComparer { let projected_x = self.projection.bind(py).call1((x,))?; let projected_y = self.projection.bind(py).call1((y,))?; - // Compare the projected values using the comparer + // Compare the projected values using the comparer. `__call__` is invoked + // O(n log n) times during a sort, so intern the method name to avoid + // recreating the "Compare" string on every comparison. let result = self .comparer .bind(py) - .call_method1("Compare", (projected_x, projected_y))?; + .call_method1(intern!(py, "Compare"), (projected_x, projected_y))?; result.extract() } } diff --git a/src/fable-library-py/tests-benchmark/test_bench_array.py b/src/fable-library-py/tests-benchmark/test_bench_array.py index 6f330e8cb9..c3e4709d84 100644 --- a/src/fable-library-py/tests-benchmark/test_bench_array.py +++ b/src/fable-library-py/tests-benchmark/test_bench_array.py @@ -15,8 +15,11 @@ _T = TypeVar("_T") -# Use Array as an alias for FSharpArray -Array = array.FSharpArray[_T] +# Use Array as an alias for FSharpArray. Must stay unsubscripted: subscripting +# (FSharpArray[_T]) now resolves to a concrete typed subclass (e.g. GenericArray) +# whose __new__ rejects the array_type kwarg, so it can't be used as a constructor. +# Subscripting is still fine in annotation position (Array[Any]). +Array = array.FSharpArray # Creation benchmarks @@ -97,7 +100,7 @@ def python_map(): def test_fsharp_map_benchmark(benchmark: BenchmarkFixture, size: int) -> None: """Benchmark map operation on FSharpArray.""" # Use Generic array type for regular Python lists - fs_array: Array[Any] = Array[Any](array_type="Generic", elements=list(range(size))) + fs_array: Array[Any] = Array(array_type="Generic", elements=list(range(size))) def fsharp_map(): return fs_array.map(lambda x: x * 2) @@ -377,3 +380,125 @@ def create_fs_array() -> Array[int32]: assert len(fs_arr) == 1000 assert fs_arr[0] == 0 assert fs_arr[1] == 1 + + +# Aggregate benchmarks (sum / average / min / max / contains) +# +# These aggregates take an adder/averager/comparer object and today loop in Rust +# calling a Python method (GetZero/Add/DivideByInt/Compare) per element even on +# unboxed numeric storage. They gate the Phase 1 native fast path: the same +# benchmark runs before/after, and the typed variants should improve sharply once +# the numeric fast path lands while Generic stays on the callback path. + + +class _Adder: + """Minimal IGenericAdder for numeric elements (elements are plain int/float).""" + + def __init__(self, zero: Any) -> None: + self._zero = zero + + def GetZero(self) -> Any: + return self._zero + + def Add(self, a: Any, b: Any) -> Any: + return a + b + + +class _Averager(_Adder): + """Minimal IGenericAverager: adds DivideByInt to the adder protocol.""" + + def DivideByInt(self, a: Any, n: int) -> Any: + return a / n + + +class _Comparer: + """Minimal IComparer using Python's built-in ordering.""" + + def Compare(self, a: Any, b: Any) -> int: + return (a > b) - (a < b) + + +@pytest.mark.parametrize(("array_type", "zero"), [("Int32", 0), ("Float64", 0.0), ("Generic", 0)]) +def test_array_sum_benchmark(benchmark: BenchmarkFixture, array_type: str, zero: Any) -> None: + """Benchmark sum over typed vs generic arrays.""" + size = 1000 + arr: Array[Any] = Array(array_type=array_type, elements=list(range(size))) # type: ignore + adder = _Adder(zero) + + benchmark.group = "Array sum (size=1000)" + benchmark.name = array_type + result = benchmark(lambda: arr.sum(adder)) + assert result == sum(range(size)) + + +@pytest.mark.parametrize(("array_type", "zero"), [("Int32", 0), ("Float64", 0.0), ("Generic", 0)]) +def test_array_average_benchmark(benchmark: BenchmarkFixture, array_type: str, zero: Any) -> None: + """Benchmark average over typed vs generic arrays.""" + size = 1000 + arr: Array[Any] = Array(array_type=array_type, elements=list(range(size))) # type: ignore + averager = _Averager(zero) + + benchmark.group = "Array average (size=1000)" + benchmark.name = array_type + result = benchmark(lambda: arr.average(averager)) + total = sum(range(size)) + # Integer element types truncate the average (int32(a / n)); floats/generic don't. + expected = total // size if array_type == "Int32" else total / size + # result may be a numeric wrapper (Int32/Float64); compare as plain floats. + assert float(result) == pytest.approx(float(expected)) + + +@pytest.mark.parametrize("array_type", ["Int32", "Float64", "Generic"]) +def test_array_max_benchmark(benchmark: BenchmarkFixture, array_type: str) -> None: + """Benchmark max over typed vs generic arrays.""" + size = 1000 + arr: Array[Any] = Array(array_type=array_type, elements=list(range(size))) # type: ignore + comparer = _Comparer() + + benchmark.group = "Array max (size=1000)" + benchmark.name = array_type + result = benchmark(lambda: arr.max(comparer)) + assert result == size - 1 + + +@pytest.mark.parametrize("array_type", ["Int32", "Float64", "Generic"]) +def test_array_min_benchmark(benchmark: BenchmarkFixture, array_type: str) -> None: + """Benchmark min over typed vs generic arrays.""" + size = 1000 + arr: Array[Any] = Array(array_type=array_type, elements=list(range(size))) # type: ignore + comparer = _Comparer() + + benchmark.group = "Array min (size=1000)" + benchmark.name = array_type + result = benchmark(lambda: arr.min(comparer)) + assert result == 0 + + +@pytest.mark.parametrize("array_type", ["Int32", "Float64", "Generic"]) +def test_array_contains_benchmark(benchmark: BenchmarkFixture, array_type: str) -> None: + """Benchmark contains (worst case: element absent) over typed vs generic arrays.""" + size = 1000 + arr: Array[Any] = Array(array_type=array_type, elements=list(range(size))) # type: ignore + # Absent value forces a full scan. + missing = size + 1 + + benchmark.group = "Array contains (size=1000, absent)" + benchmark.name = array_type + result = benchmark(lambda: arr.contains(missing, None)) + assert result is False + + +# Generic-vs-typed map (gates the Phase 2 Arc removal on generic storage) + + +@pytest.mark.parametrize("array_type", ["Int32", "Generic"]) +def test_array_map_typed_vs_generic_benchmark(benchmark: BenchmarkFixture, array_type: str) -> None: + """Benchmark map on typed vs generic storage (same callback).""" + size = 1000 + arr: Array[Any] = Array(array_type=array_type, elements=list(range(size))) # type: ignore + + benchmark.group = "Array map typed-vs-generic (size=1000)" + benchmark.name = array_type + result = benchmark(lambda: arr.map(lambda x: x + 1)) + assert len(result) == size + assert result[0] == 1 diff --git a/src/fable-library-py/tests/test_array.py b/src/fable-library-py/tests/test_array.py index 4daefdaf9f..8278466ed9 100644 --- a/src/fable-library-py/tests/test_array.py +++ b/src/fable-library-py/tests/test_array.py @@ -1439,3 +1439,199 @@ class Data(BaseModel): restored = Data.model_validate_json(json_str) assert list(restored.values) == [10, 20, 30] assert type(restored.values).__name__ == "FSharpArray" + + +# --------------------------------------------------------------------------- +# Native numeric aggregate fast paths (sum / average / min / max / contains) +# +# These assert the native typed-storage fast path is equivalent to the +# Python-callback path, which is exercised by building a Generic array from the +# SAME wrapper objects: the callback path's wrapper arithmetic wraps identically, +# so it is a faithful oracle for the native path. +# --------------------------------------------------------------------------- + +# Numeric element strategies keyed by array type (reuse the module-level ones). +_NUMERIC_ARRAY_TYPES = ["Int8", "UInt8", "Int16", "UInt16", "Int32", "UInt32", "Int64", "UInt64", "Float32", "Float64"] +_INT_ARRAY_TYPES = ["Int8", "UInt8", "Int16", "UInt16", "Int32", "UInt32", "Int64", "UInt64"] + +_ZERO = { + "Int8": sbyte(0), + "UInt8": byte(0), + "Int16": int16(0), + "UInt16": uint16(0), + "Int32": int32(0), + "UInt32": uint32(0), + "Int64": int64(0), + "UInt64": uint64(0), + "Float32": float32(0.0), + "Float64": float64(0.0), +} + + +class _Adder: + """Minimal IGenericAdder; GetZero returns a typed wrapper so callback-path + arithmetic wraps at the element width (matching the native path).""" + + def __init__(self, zero: Any) -> None: + self._zero = zero + + def GetZero(self) -> Any: + return self._zero + + def Add(self, a: Any, b: Any) -> Any: + return a + b + + +class _Averager(_Adder): + def DivideByInt(self, a: Any, n: int) -> Any: + return a / n + + +class _Comparer: + def Compare(self, a: Any, b: Any) -> int: + return (a > b) - (a < b) + + +def _typed_and_generic(array_type: str, elements: list[Any]) -> tuple[Any, Any]: + typed = Array(array_type=array_type, elements=list(elements)) + generic = Array(array_type="Generic", elements=list(elements)) + return typed, generic + + +def _agg_eq(a: Any, b: Any) -> bool: + """Equality that treats two NaNs as equal (float inf + -inf sums produce NaN).""" + import math + + fa, fb = float(a), float(b) + if math.isnan(fa) and math.isnan(fb): + return True + return bool(a == b) + + +@pytest.mark.parametrize("array_type", _NUMERIC_ARRAY_TYPES) +@given(data=st.data()) +def test_native_sum_matches_callback(array_type: str, data: st.DataObject) -> None: + """Native sum on typed storage equals the callback path (value and type).""" + elements = data.draw(st.lists(array_types[array_type], min_size=0, max_size=64)) + typed, generic = _typed_and_generic(array_type, elements) + adder = _Adder(_ZERO[array_type]) + typed_sum = typed.sum(adder) + generic_sum = generic.sum(adder) + assert _agg_eq(typed_sum, generic_sum) + # Result wrapper type is preserved (Int32 sum -> Int32, etc.) + assert type(typed_sum).__name__ == array_type + + +@pytest.mark.parametrize("array_type", _NUMERIC_ARRAY_TYPES) +@given(data=st.data()) +def test_native_average_matches_callback(array_type: str, data: st.DataObject) -> None: + """Native average on typed storage equals the callback path.""" + elements = data.draw(st.lists(array_types[array_type], min_size=1, max_size=64)) + typed, generic = _typed_and_generic(array_type, elements) + averager = _Averager(_ZERO[array_type]) + assert _agg_eq(typed.average(averager), generic.average(averager)) + + +@pytest.mark.parametrize("array_type", _NUMERIC_ARRAY_TYPES) +def test_native_average_empty_raises(array_type: str) -> None: + """Average of an empty typed array raises, like the callback path.""" + empty = Array(array_type=array_type, elements=[]) + with pytest.raises(Exception): + empty.average(_Averager(_ZERO[array_type])) + + +@pytest.mark.parametrize("array_type", _INT_ARRAY_TYPES) +@given(data=st.data()) +def test_native_min_max_matches_callback(array_type: str, data: st.DataObject) -> None: + """Native min/max on integer storage equals the callback path (by value).""" + elements = data.draw(st.lists(array_types[array_type], min_size=1, max_size=64)) + typed, generic = _typed_and_generic(array_type, elements) + comparer = _Comparer() + assert typed.max(comparer) == generic.max(comparer) + assert typed.min(comparer) == generic.min(comparer) + + +@pytest.mark.parametrize("array_type", _INT_ARRAY_TYPES) +def test_native_min_max_empty_raises(array_type: str) -> None: + """Min/max of an empty typed array raises, like the callback path.""" + empty = Array(array_type=array_type, elements=[]) + with pytest.raises(Exception): + empty.max(_Comparer()) + with pytest.raises(Exception): + empty.min(_Comparer()) + + +@pytest.mark.parametrize("array_type", _NUMERIC_ARRAY_TYPES) +@given(data=st.data()) +def test_native_contains_matches_callback(array_type: str, data: st.DataObject) -> None: + """Native contains on typed storage equals the callback path.""" + elements = data.draw(st.lists(array_types[array_type], min_size=0, max_size=64)) + typed, generic = _typed_and_generic(array_type, elements) + # Draw a probe value of the same element type (present or absent). + value = data.draw(array_types[array_type]) + assert typed.contains(value, None) == generic.contains(value, None) + + +# Cross-type probes: the probe value is not representable as the element type, so +# the native fast path bails out (`try_native_contains` returns None) and the +# scan falls back to `rich_compare`, preserving Python's cross-type equality +# (2 == 2.0, True == 1). The typed (native) path must agree with the generic +# (callback) oracle in every case. +@pytest.mark.parametrize( + ("array_type", "elements", "probe", "expected"), + [ + # int storage probed with a float that equals / doesn't equal an element + ("Int32", [1, 2, 3], 2.0, True), + ("Int32", [1, 2, 3], 2.5, False), + # int storage probed with a bool (True == 1 in Python) + ("Int32", [0, 1], True, True), + # float storage probed with an int + ("Float64", [1.0, 2.0], 2, True), + ("Float64", [1.0, 2.0], 3, False), + # bool storage probed with an int (extract:: rejects a plain int) + ("Bool", [True, False], 1, True), + ("Bool", [True, False], 0, True), + ("Bool", [True], 2, False), + ], +) +def test_native_contains_cross_type_fallback(array_type: str, elements: list[Any], probe: Any, expected: bool) -> None: + """Cross-type probes fall back to rich_compare; native == callback == Python.""" + typed = Array(array_type=array_type, elements=list(elements)) + generic = Array(array_type="Generic", elements=list(elements)) + assert typed.contains(probe, None) == expected + assert typed.contains(probe, None) == generic.contains(probe, None) + + +# --------------------------------------------------------------------------- +# Value semantics for generic (PyObject) storage. +# +# Generic storage is a plain Vec: cloning deep-copies element references, so a +# copy/sort never aliases the source's storage. These guard against the previous +# Arc sharing where sort()/copy() mutated the source in place. +# --------------------------------------------------------------------------- + + +def test_generic_copy_is_independent() -> None: + arr = Array(array_type="Generic", elements=[3, 1, 2]) + copied = arr.copy() + copied[0] = 99 + assert list(arr) == [3, 1, 2], "mutating the copy must not affect the source" + assert list(copied) == [99, 1, 2] + + +def test_generic_sort_does_not_mutate_source() -> None: + arr = Array(array_type="Generic", elements=[3, 1, 2]) + result = arr.sort(_Comparer()) + assert list(arr) == [3, 1, 2], "sort() must not mutate the source array" + assert list(result) == [1, 2, 3] + + +@given(elements=st.lists(st.integers(), min_size=0, max_size=64)) +def test_generic_copy_round_trip(elements: list[int]) -> None: + arr = Array(array_type="Generic", elements=list(elements)) + copied = arr.copy() + assert list(copied) == list(elements) + # Independent: appending/replacing in the copy leaves the source untouched. + if elements: + copied[0] = "changed" + assert list(arr) == list(elements)