diff --git a/.github/workflows/build.yaml b/.github/workflows/build.yaml index 8a25739903..7fa5b39a42 100644 --- a/.github/workflows/build.yaml +++ b/.github/workflows/build.yaml @@ -152,7 +152,7 @@ jobs: - name: WebAssembly Build run: pnpm run build --ci env: - PACKAGE: "server,client,viewer,viewer-datagrid,viewer-charts,workspace,react" + PACKAGE: "server,client,viewer,viewer-datagrid,viewer-charts,react" # PSP_USE_CCACHE: 1 - uses: actions/upload-artifact@v4 @@ -166,7 +166,6 @@ jobs: rust/perspective-viewer/src packages/viewer-charts/dist packages/viewer-datagrid/dist - packages/workspace/dist packages/react/dist # ,-,---. . . .-,--. . . @@ -669,7 +668,7 @@ jobs: id: run_tests run: pnpm run test -- --fetch-snapshots env: - PACKAGE: "server,client,viewer,viewer-datagrid,viewer-charts,workspace,react" + PACKAGE: "server,client,viewer,viewer-datagrid,viewer-charts,react" PSP_SNAPSHOT_REPO: ${{ vars.PSP_SNAPSHOT_REPO }} PSP_SNAPSHOT_TOKEN: ${{ secrets.PSP_SNAPSHOT_TOKEN }} PSP_SNAPSHOT_REF: ${{ github.head_ref || github.ref_name }} @@ -1047,9 +1046,6 @@ jobs: - run: pnpm pack --pack-destination=../.. working-directory: ./packages/viewer-charts - - run: pnpm pack --pack-destination=../.. - working-directory: ./packages/workspace - - run: pnpm pack --pack-destination=../.. working-directory: ./packages/react diff --git a/rust/perspective-js/src/rust/typed_array.rs b/rust/perspective-js/src/rust/typed_array.rs index ad195d797e..a14422ab6b 100644 --- a/rust/perspective-js/src/rust/typed_array.rs +++ b/rust/perspective-js/src/rust/typed_array.rs @@ -12,9 +12,9 @@ use std::io::Cursor; -use arrow_array::Array as _; use arrow_array::cast::AsArray; use arrow_array::types::*; +use arrow_array::{Array as _, ArrowPrimitiveType, PrimitiveArray}; use arrow_ipc::reader::StreamReader; use arrow_schema::{DataType, TimeUnit}; use js_sys::{Array, Function, JsString, Uint8Array}; @@ -49,6 +49,31 @@ impl From for ViewWindow { } } +fn zero_invalid_slots(arr: &PrimitiveArray) { + let Some(nulls) = arr.nulls() else { return }; + let ptr = arr.values().as_ptr() as *mut T::Native; + let chunks = nulls.inner().bit_chunks(); + let mut base = 0; + for chunk in chunks.iter() { + if chunk != u64::MAX { + for bit in 0..64 { + if chunk & (1 << bit) == 0 { + unsafe { ptr.add(base + bit).write(T::default_value()) }; + } + } + } + + base += 64; + } + + let rem = chunks.remainder_bits(); + for bit in 0..chunks.remainder_len() { + if rem & (1 << bit) == 0 { + unsafe { ptr.add(base + bit).write(T::default_value()) }; + } + } +} + /// Decode an Arrow IPC batch and call `callback` once with all columns. /// /// Callback signature: @@ -83,13 +108,23 @@ pub(crate) async fn decode_and_call( let js_validities = Array::new_with_length(num_cols as u32); let js_dicts = Array::new_with_length(num_cols as u32); - // Storage for allocated conversion buffers. These MUST outlive the - // callback because `js_sys::*Array::view()` creates zero-copy views - // into their heap memory. Using `Box<[T]>` (rather than `Vec`) - // yields a stable pointer that won't move when the outer Vec grows. + // Storage for type-conversion buffers (Int64/Date32/Timestamp and + // `float32` narrowing). These MUST outlive the callback because + // `js_sys::*Array::view()` creates zero-copy views into their heap + // memory. Using `Box<[T]>` (rather than `Vec`) yields a stable + // data pointer that won't move when the outer Vec grows, so a view + // created before the push stays valid. let mut f32_storage: Vec> = Vec::new(); let mut f64_storage: Vec> = Vec::new(); + // The bytes under a NULL slot in the source Arrow are UNDEFINED — + // the perspective engine zero-fills them but e.g. DuckDB's Arrow + // output leaves NaN (which, unlike the garbage a consumer merely + // *displays* wrong, poisons any consumer that aggregates, e.g. the + // treemap's bottom-up value sums). `zero_invalid_slots` normalizes + // every column in place — forcing invalid slots to 0 so all backends + // present the same value contract — without giving up the zero-copy + // view. for col_idx in 0..num_cols { let field = schema.field(col_idx); let col = batch.column(col_idx); @@ -99,32 +134,41 @@ pub(crate) async fn decode_and_call( match col.data_type() { DataType::UInt32 => { - let vals = col.as_primitive::().values(); - let arr = unsafe { js_sys::Uint32Array::view(vals.as_ref()) }; + let typed = col.as_primitive::(); + zero_invalid_slots(typed); + let arr = unsafe { js_sys::Uint32Array::view(typed.values().as_ref()) }; js_values.set(col_idx as u32, arr.into()); js_dicts.set(col_idx as u32, JsValue::NULL); }, DataType::Int32 => { - let vals = col.as_primitive::().values(); - let arr = unsafe { js_sys::Int32Array::view(vals.as_ref()) }; + let typed = col.as_primitive::(); + zero_invalid_slots(typed); + let arr = unsafe { js_sys::Int32Array::view(typed.values().as_ref()) }; js_values.set(col_idx as u32, arr.into()); js_dicts.set(col_idx as u32, JsValue::NULL); }, DataType::Float32 => { - let vals = col.as_primitive::().values(); - let arr = unsafe { js_sys::Float32Array::view(vals.as_ref()) }; + let typed = col.as_primitive::(); + zero_invalid_slots(typed); + let arr = unsafe { js_sys::Float32Array::view(typed.values().as_ref()) }; js_values.set(col_idx as u32, arr.into()); js_dicts.set(col_idx as u32, JsValue::NULL); }, DataType::Float64 => { + let typed = col.as_primitive::(); + zero_invalid_slots(typed); if float32 { - let vals = col.as_primitive::().values(); - f32_storage.push(vals.iter().map(|&v| v as f32).collect()); + let vals: Box<[f32]> = typed.values().iter().map(|&v| v as f32).collect(); + + let arr = unsafe { js_sys::Float32Array::view(&vals) }; + f32_storage.push(vals); + js_values.set(col_idx as u32, arr.into()); } else { - let vals = col.as_primitive::().values(); - let arr = unsafe { js_sys::Float64Array::view(vals.as_ref()) }; + let arr = unsafe { js_sys::Float64Array::view(typed.values().as_ref()) }; + js_values.set(col_idx as u32, arr.into()); } + js_dicts.set(col_idx as u32, JsValue::NULL); }, DataType::Date32 => { @@ -133,32 +177,51 @@ pub(crate) async fn decode_and_call( // timestamps, so the `float32` flag is intentionally ignored // for date/timestamp columns. let typed = col.as_primitive::(); - f64_storage.push( - typed - .values() - .iter() - .map(|&v| v as f64 * 86_400_000.0) - .collect(), - ); + zero_invalid_slots(typed); + let vals: Box<[f64]> = typed + .values() + .iter() + .map(|&v| v as f64 * 86_400_000.0) + .collect(); + + let arr = unsafe { js_sys::Float64Array::view(&vals) }; + f64_storage.push(vals); + js_values.set(col_idx as u32, arr.into()); js_dicts.set(col_idx as u32, JsValue::NULL); }, DataType::Timestamp(TimeUnit::Millisecond, _) => { let typed = col.as_primitive::(); - f64_storage.push(typed.values().iter().map(|&v| v as f64).collect()); + zero_invalid_slots(typed); + let vals: Box<[f64]> = typed.values().iter().map(|&v| v as f64).collect(); + + let arr = unsafe { js_sys::Float64Array::view(&vals) }; + f64_storage.push(vals); + js_values.set(col_idx as u32, arr.into()); js_dicts.set(col_idx as u32, JsValue::NULL); }, DataType::Int64 => { let typed = col.as_primitive::(); + zero_invalid_slots(typed); if float32 { - f32_storage.push(typed.values().iter().map(|&v| v as f32).collect()); + let vals: Box<[f32]> = typed.values().iter().map(|&v| v as f32).collect(); + + let arr = unsafe { js_sys::Float32Array::view(&vals) }; + f32_storage.push(vals); + js_values.set(col_idx as u32, arr.into()); } else { - f64_storage.push(typed.values().iter().map(|&v| v as f64).collect()); + let vals: Box<[f64]> = typed.values().iter().map(|&v| v as f64).collect(); + + let arr = unsafe { js_sys::Float64Array::view(&vals) }; + f64_storage.push(vals); + js_values.set(col_idx as u32, arr.into()); } + js_dicts.set(col_idx as u32, JsValue::NULL); }, DataType::Dictionary(..) => { let dict = col.as_dictionary::(); let keys = dict.keys(); + zero_invalid_slots(keys); let arr = unsafe { js_sys::Int32Array::view(keys.values().as_ref()) }; js_values.set(col_idx as u32, arr.into()); @@ -188,35 +251,6 @@ pub(crate) async fn decode_and_call( ); } - // Second pass: fill in value views for columns backed by f32_storage / - // f64_storage. The Box<[T]> buffers are heap-allocated and stable; their - // data pointers remain valid even as the outer Vec grows. - let mut f32_idx = 0; - let mut f64_idx = 0; - for col_idx in 0..num_cols { - let col = batch.column(col_idx); - let uses_f32_storage = matches!( - (col.data_type(), float32), - (DataType::Float64, true) | (DataType::Int64, true), - ); - let uses_f64_storage = matches!( - (col.data_type(), float32), - (DataType::Date32, _) - | (DataType::Timestamp(TimeUnit::Millisecond, _), _) - | (DataType::Int64, false), - ); - - if uses_f32_storage { - let arr = unsafe { js_sys::Float32Array::view(&f32_storage[f32_idx]) }; - js_values.set(col_idx as u32, arr.into()); - f32_idx += 1; - } else if uses_f64_storage { - let arr = unsafe { js_sys::Float64Array::view(&f64_storage[f64_idx]) }; - js_values.set(col_idx as u32, arr.into()); - f64_idx += 1; - } - } - let ret = callback.call4( &JsValue::UNDEFINED, &js_names.into(), diff --git a/rust/perspective-js/test/js/duckdb/typed_arrays.spec.js b/rust/perspective-js/test/js/duckdb/typed_arrays.spec.js new file mode 100644 index 0000000000..9862497e31 --- /dev/null +++ b/rust/perspective-js/test/js/duckdb/typed_arrays.spec.js @@ -0,0 +1,91 @@ +// ┏━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┓ +// ┃ ██████ ██████ ██████ █ █ █ █ █ █▄ ▀███ █ ┃ +// ┃ ▄▄▄▄▄█ █▄▄▄▄▄ ▄▄▄▄▄█ ▀▀▀▀▀█▀▀▀▀▀ █ ▀▀▀▀▀█ ████████▌▐███ ███▄ ▀█ █ ▀▀▀▀▀ ┃ +// ┃ █▀▀▀▀▀ █▀▀▀▀▀ █▀██▀▀ ▄▄▄▄▄ █ ▄▄▄▄▄█ ▄▄▄▄▄█ ████████▌▐███ █████▄ █ ▄▄▄▄▄ ┃ +// ┃ █ ██████ █ ▀█▄ █ ██████ █ ███▌▐███ ███████▄ █ ┃ +// ┣━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┫ +// ┃ Copyright (c) 2017, the Perspective Authors. ┃ +// ┃ ╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌ ┃ +// ┃ This file is part of the Perspective library, distributed under the terms ┃ +// ┃ of the [Apache License 2.0](https://www.apache.org/licenses/LICENSE-2.0). ┃ +// ┗━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┛ + +import { test, expect } from "@perspective-dev/test"; +import { describeDuckDB } from "./setup.js"; + +/** + * Arrow validity-bitmap test — bit `i` of the LSB-ordered bitfield. + */ +function isValid(validity, i) { + return !validity || !!((validity[i >> 3] >> i % 8) & 1); +} + +describeDuckDB("typed_arrays", (getClient) => { + test("invalid slots read 0, never NaN, in a sparse split_by pivot", async function () { + const table = await getClient().open_table("memory.superstore"); + const view = await table.view({ + columns: ["Sales"], + group_by: ["Region", "City"], + split_by: ["Category"], + aggregates: { Sales: "sum" }, + group_rollup_mode: "flat", + }); + + let invalidSlots = 0; + let checkedCols = 0; + await view.with_typed_arrays( + {}, + (names, values, validities, dictionaries) => { + for (let c = 0; c < names.length; c++) { + if (names[c].startsWith("__") || dictionaries[c]) { + continue; + } + + checkedCols++; + const vals = values[c]; + const validity = validities[c]; + for (let i = 0; i < vals.length; i++) { + expect(Number.isNaN(vals[i])).toBe(false); + if (!isValid(validity, i)) { + invalidSlots++; + expect(vals[i]).toBe(0); + } + } + } + }, + ); + + expect(checkedCols).toBe(3); + expect(invalidSlots).toBeGreaterThan(0); + await view.delete(); + }); + + test("invalid dictionary keys are clamped in-bounds", async function () { + const table = await getClient().open_table("memory.superstore"); + const view = await table.view({ + columns: ["Sales"], + group_by: ["Region", "City"], + aggregates: { Sales: "sum" }, + }); + + await view.with_typed_arrays( + {}, + (names, values, validities, dictionaries) => { + for (let c = 0; c < names.length; c++) { + const dict = dictionaries[c]; + if (!dict) { + continue; + } + + const keys = values[c]; + for (let i = 0; i < keys.length; i++) { + expect(keys[i]).toBeGreaterThanOrEqual(0); + expect(keys[i]).toBeLessThan(dict.length); + } + } + }, + ); + + await view.delete(); + }); +}); diff --git a/rust/perspective-js/test/js/leaks.spec.js b/rust/perspective-js/test/js/leaks.spec.js index 5cfb244184..5540a426f6 100644 --- a/rust/perspective-js/test/js/leaks.spec.js +++ b/rust/perspective-js/test/js/leaks.spec.js @@ -101,26 +101,21 @@ test.describe("leaks", function () { await table.delete(); }); - test.skip("OG - to_columns_string does not leak", async () => { - const table = await perspective.table(arr.slice()); - const view = await table.view({ group_by: ["State"] }); - await leak_test(async function () { - let json = await view.to_columns_string(); - expect(json.length).toEqual(6722); - }); - await view.delete(); - await table.delete(); - }); - - // The length of this string changed due to + // The length of this string changes due to // some of the trailing fractional digits in the floats. test("to_columns_string does not leak", async () => { const table = await perspective.table(arr.slice()); const view = await table.view({ group_by: ["State"] }); + let len; await leak_test(async function () { let json = await view.to_columns_string(); - expect(json.length).toEqual(6669); + if (!len) { + len = json.length; + } else { + expect(json.length).toEqual(len); + } }); + await view.delete(); await table.delete(); }); diff --git a/rust/perspective-server/build.rs b/rust/perspective-server/build.rs index 975bbaabcc..c8167c16bf 100644 --- a/rust/perspective-server/build.rs +++ b/rust/perspective-server/build.rs @@ -142,6 +142,11 @@ fn cmake_link_deps(cmake_build_dir: &Path) -> Result<(), std::io::Error> { cmake_build_dir.display() ); + if cfg!(windows) { + println!("cargo:rustc-link-lib=shell32"); + println!("cargo:rustc-link-lib=ole32"); + } + // println!("cargo:warning=MESSAGE {}/build", cmake_build_dir.display()); println!("cargo:rustc-link-lib=static=psp"); link_cmake_static_archives(cmake_build_dir)?; diff --git a/rust/perspective-server/clean.mjs b/rust/perspective-server/clean.mjs index 49a5253931..4f6d3bd668 100644 --- a/rust/perspective-server/clean.mjs +++ b/rust/perspective-server/clean.mjs @@ -13,3 +13,4 @@ import * as fs from "node:fs"; fs.rmSync("dist", { recursive: true, force: true }); +fs.rmSync("build", { recursive: true, force: true }); diff --git a/rust/perspective-server/cmake/modules/FindInstallDependency.cmake b/rust/perspective-server/cmake/modules/FindInstallDependency.cmake index e4921584be..7378a9d618 100644 --- a/rust/perspective-server/cmake/modules/FindInstallDependency.cmake +++ b/rust/perspective-server/cmake/modules/FindInstallDependency.cmake @@ -51,10 +51,6 @@ function(psp_build_dep name cmake_file) set(ARROW_NO_EXPORT ON) set(ARROW_CXXFLAGS " -Wno-documentation ") set(ARROW_DEPENDENCY_SOURCE "BUNDLED" CACHE STRING "override arrow's dependency source" FORCE) - # if (PSP_ENABLE_WASM) - # set(ARROW_CXX_FLAGS_RELEASE " -msimd128 -mbulk-memory -mrelaxed-simd -s MEMORY64=1 " CACHE STRING "override arrow's dependency source" FORCE) - # set(ARROW_C_FLAGS_RELEASE " -msimd128 -mbulk-memory -mrelaxed-simd -s MEMORY64=1 " CACHE STRING "override arrow's dependency source" FORCE ) - # endif() if(PSP_WASM_BUILD) set(ARROW_ENABLE_THREADING OFF) else() diff --git a/rust/perspective-server/cpp/perspective/CMakeLists.txt b/rust/perspective-server/cpp/perspective/CMakeLists.txt index 029ea01403..6150b8a671 100644 --- a/rust/perspective-server/cpp/perspective/CMakeLists.txt +++ b/rust/perspective-server/cpp/perspective/CMakeLists.txt @@ -263,6 +263,7 @@ if(PSP_WASM_BUILD) -mbulk-memory \ -msimd128 \ -mrelaxed-simd \ + -fopenmp-simd \ -flto \ --emit-tsd=perspective-server.d.ts \ ") @@ -371,6 +372,12 @@ if(PSP_PYODIDE) string(APPEND CMAKE_CXX_FLAGS " -fexceptions") endif() +if(PSP_WASM_BUILD AND NOT CMAKE_BUILD_TYPE_LOWER STREQUAL debug) + set(PSP_DEP_SIMD_FLAGS " -flto -mbulk-memory -msimd128 -mrelaxed-simd ") + string(APPEND CMAKE_C_FLAGS "${PSP_DEP_SIMD_FLAGS}") + string(APPEND CMAKE_CXX_FLAGS "${PSP_DEP_SIMD_FLAGS}") +endif() + # Build (read: download and extract) header-only dependencies from external sources set(all_deps_INCLUDE_DIRS "") psp_build_dep("date" "${PSP_CMAKE_MODULE_PATH}/date.txt.in") diff --git a/rust/perspective-viewer/test/js/stability/export.spec.ts b/rust/perspective-viewer/test/js/stability/export.spec.ts index f0b3f1f5af..81621df124 100644 --- a/rust/perspective-viewer/test/js/stability/export.spec.ts +++ b/rust/perspective-viewer/test/js/stability/export.spec.ts @@ -51,6 +51,6 @@ test.describe("Single-Threaded Engine", () => { }); // Superstore as a CSV - expect(value).toEqual(836); + expect(value).toEqual(821); }); });