From 41f65749c7c2528f115a15c135122a454349fb77 Mon Sep 17 00:00:00 2001 From: Andy Grove Date: Tue, 10 Mar 2026 17:59:25 -0600 Subject: [PATCH 1/5] perf: use direct aligned pointer reads in SparkUnsafeObject accessors Replace slice construction + try_into().unwrap() + from_le_bytes() with direct pointer dereferences in all SparkUnsafeObject trait accessors. Both SparkUnsafeRow and SparkUnsafeArray guarantee natural alignment for field access: UnsafeRow fields are at 8-byte aligned offsets (bitset width is multiple of 8, each slot is 8 bytes, JVM allocates aligned memory), and UnsafeArray elements are at naturally aligned offsets (header is 8-byte aligned, elements are at element_size stride). This eliminates three layers of abstraction per read (from_raw_parts, try_into().unwrap(), from_le_bytes) on the hottest path in row-to- columnar conversion. --- .../execution/shuffle/spark_unsafe/list.rs | 4 ++-- .../src/execution/shuffle/spark_unsafe/row.rs | 24 +++++++------------ 2 files changed, 10 insertions(+), 18 deletions(-) diff --git a/native/core/src/execution/shuffle/spark_unsafe/list.rs b/native/core/src/execution/shuffle/spark_unsafe/list.rs index 72610d2d82..7cf5fa6b05 100644 --- a/native/core/src/execution/shuffle/spark_unsafe/list.rs +++ b/native/core/src/execution/shuffle/spark_unsafe/list.rs @@ -109,8 +109,8 @@ impl SparkUnsafeArray { // SAFETY: addr points to valid Spark UnsafeArray data from the JVM. // The first 8 bytes contain the element count as a little-endian i64. debug_assert!(addr != 0, "SparkUnsafeArray::new: null address"); - let slice: &[u8] = unsafe { std::slice::from_raw_parts(addr as *const u8, 8) }; - let num_elements = i64::from_le_bytes(slice.try_into().unwrap()); + // SAFETY: Spark UnsafeArray stores element count as the first 8 bytes, 8-byte aligned. + let num_elements = unsafe { *(addr as *const i64) }; if num_elements < 0 { panic!("Negative number of elements: {num_elements}"); diff --git a/native/core/src/execution/shuffle/spark_unsafe/row.rs b/native/core/src/execution/shuffle/spark_unsafe/row.rs index 6b41afae8d..f64a16f86a 100644 --- a/native/core/src/execution/shuffle/spark_unsafe/row.rs +++ b/native/core/src/execution/shuffle/spark_unsafe/row.rs @@ -106,8 +106,7 @@ pub trait SparkUnsafeObject { let addr = self.get_element_offset(index, 1); // SAFETY: addr points to valid element data (1 byte) within the row/array region. debug_assert!(!addr.is_null(), "get_byte: null pointer at index {index}"); - let slice: &[u8] = unsafe { std::slice::from_raw_parts(addr, 1) }; - i8::from_le_bytes(slice.try_into().unwrap()) + unsafe { *(addr as *const i8) } } /// Returns short value at the given index of the object. @@ -116,8 +115,7 @@ pub trait SparkUnsafeObject { let addr = self.get_element_offset(index, 2); // SAFETY: addr points to valid element data (2 bytes) within the row/array region. debug_assert!(!addr.is_null(), "get_short: null pointer at index {index}"); - let slice: &[u8] = unsafe { std::slice::from_raw_parts(addr, 2) }; - i16::from_le_bytes(slice.try_into().unwrap()) + unsafe { *(addr as *const i16) } } /// Returns integer value at the given index of the object. @@ -126,8 +124,7 @@ pub trait SparkUnsafeObject { let addr = self.get_element_offset(index, 4); // SAFETY: addr points to valid element data (4 bytes) within the row/array region. debug_assert!(!addr.is_null(), "get_int: null pointer at index {index}"); - let slice: &[u8] = unsafe { std::slice::from_raw_parts(addr, 4) }; - i32::from_le_bytes(slice.try_into().unwrap()) + unsafe { *(addr as *const i32) } } /// Returns long value at the given index of the object. @@ -136,8 +133,7 @@ pub trait SparkUnsafeObject { let addr = self.get_element_offset(index, 8); // SAFETY: addr points to valid element data (8 bytes) within the row/array region. debug_assert!(!addr.is_null(), "get_long: null pointer at index {index}"); - let slice: &[u8] = unsafe { std::slice::from_raw_parts(addr, 8) }; - i64::from_le_bytes(slice.try_into().unwrap()) + unsafe { *(addr as *const i64) } } /// Returns float value at the given index of the object. @@ -146,8 +142,7 @@ pub trait SparkUnsafeObject { let addr = self.get_element_offset(index, 4); // SAFETY: addr points to valid element data (4 bytes) within the row/array region. debug_assert!(!addr.is_null(), "get_float: null pointer at index {index}"); - let slice: &[u8] = unsafe { std::slice::from_raw_parts(addr, 4) }; - f32::from_le_bytes(slice.try_into().unwrap()) + unsafe { *(addr as *const f32) } } /// Returns double value at the given index of the object. @@ -156,8 +151,7 @@ pub trait SparkUnsafeObject { let addr = self.get_element_offset(index, 8); // SAFETY: addr points to valid element data (8 bytes) within the row/array region. debug_assert!(!addr.is_null(), "get_double: null pointer at index {index}"); - let slice: &[u8] = unsafe { std::slice::from_raw_parts(addr, 8) }; - f64::from_le_bytes(slice.try_into().unwrap()) + unsafe { *(addr as *const f64) } } /// Returns string value at the given index of the object. @@ -196,8 +190,7 @@ pub trait SparkUnsafeObject { let addr = self.get_element_offset(index, 4); // SAFETY: addr points to valid element data (4 bytes) within the row/array region. debug_assert!(!addr.is_null(), "get_date: null pointer at index {index}"); - let slice: &[u8] = unsafe { std::slice::from_raw_parts(addr, 4) }; - i32::from_le_bytes(slice.try_into().unwrap()) + unsafe { *(addr as *const i32) } } /// Returns timestamp value at the given index of the object. @@ -209,8 +202,7 @@ pub trait SparkUnsafeObject { !addr.is_null(), "get_timestamp: null pointer at index {index}" ); - let slice: &[u8] = unsafe { std::slice::from_raw_parts(addr, 8) }; - i64::from_le_bytes(slice.try_into().unwrap()) + unsafe { *(addr as *const i64) } } /// Returns decimal value at the given index of the object. From 08ca4fe2b5820733f5336c0a9660b6617379e52d Mon Sep 17 00:00:00 2001 From: Andy Grove Date: Tue, 10 Mar 2026 18:57:31 -0600 Subject: [PATCH 2/5] fix: use read_unaligned for SparkUnsafeArray compatibility SparkUnsafeArray elements may not be naturally aligned (e.g., i64 elements at 4-byte-aligned offsets). Use read_unaligned() instead of direct pointer dereferences for all multi-byte accessors. This is still faster than the original from_raw_parts + try_into + from_le_bytes chain while being safe for both SparkUnsafeRow (always aligned) and SparkUnsafeArray (potentially unaligned). --- .../core/src/execution/shuffle/spark_unsafe/row.rs | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/native/core/src/execution/shuffle/spark_unsafe/row.rs b/native/core/src/execution/shuffle/spark_unsafe/row.rs index f64a16f86a..3a3055e825 100644 --- a/native/core/src/execution/shuffle/spark_unsafe/row.rs +++ b/native/core/src/execution/shuffle/spark_unsafe/row.rs @@ -115,7 +115,7 @@ pub trait SparkUnsafeObject { let addr = self.get_element_offset(index, 2); // SAFETY: addr points to valid element data (2 bytes) within the row/array region. debug_assert!(!addr.is_null(), "get_short: null pointer at index {index}"); - unsafe { *(addr as *const i16) } + unsafe { (addr as *const i16).read_unaligned() } } /// Returns integer value at the given index of the object. @@ -124,7 +124,7 @@ pub trait SparkUnsafeObject { let addr = self.get_element_offset(index, 4); // SAFETY: addr points to valid element data (4 bytes) within the row/array region. debug_assert!(!addr.is_null(), "get_int: null pointer at index {index}"); - unsafe { *(addr as *const i32) } + unsafe { (addr as *const i32).read_unaligned() } } /// Returns long value at the given index of the object. @@ -133,7 +133,7 @@ pub trait SparkUnsafeObject { let addr = self.get_element_offset(index, 8); // SAFETY: addr points to valid element data (8 bytes) within the row/array region. debug_assert!(!addr.is_null(), "get_long: null pointer at index {index}"); - unsafe { *(addr as *const i64) } + unsafe { (addr as *const i64).read_unaligned() } } /// Returns float value at the given index of the object. @@ -142,7 +142,7 @@ pub trait SparkUnsafeObject { let addr = self.get_element_offset(index, 4); // SAFETY: addr points to valid element data (4 bytes) within the row/array region. debug_assert!(!addr.is_null(), "get_float: null pointer at index {index}"); - unsafe { *(addr as *const f32) } + unsafe { (addr as *const f32).read_unaligned() } } /// Returns double value at the given index of the object. @@ -151,7 +151,7 @@ pub trait SparkUnsafeObject { let addr = self.get_element_offset(index, 8); // SAFETY: addr points to valid element data (8 bytes) within the row/array region. debug_assert!(!addr.is_null(), "get_double: null pointer at index {index}"); - unsafe { *(addr as *const f64) } + unsafe { (addr as *const f64).read_unaligned() } } /// Returns string value at the given index of the object. @@ -190,7 +190,7 @@ pub trait SparkUnsafeObject { let addr = self.get_element_offset(index, 4); // SAFETY: addr points to valid element data (4 bytes) within the row/array region. debug_assert!(!addr.is_null(), "get_date: null pointer at index {index}"); - unsafe { *(addr as *const i32) } + unsafe { (addr as *const i32).read_unaligned() } } /// Returns timestamp value at the given index of the object. @@ -202,7 +202,7 @@ pub trait SparkUnsafeObject { !addr.is_null(), "get_timestamp: null pointer at index {index}" ); - unsafe { *(addr as *const i64) } + unsafe { (addr as *const i64).read_unaligned() } } /// Returns decimal value at the given index of the object. From 09b6d40f540b8ae393fe3abfb8e0bf12abc7eca6 Mon Sep 17 00:00:00 2001 From: Andy Grove Date: Wed, 11 Mar 2026 08:41:47 -0600 Subject: [PATCH 3/5] fix: use read_unaligned for SparkUnsafeArray element count Nested arrays may not be 8-byte aligned within rows, causing a misaligned pointer dereference panic in debug builds. --- native/core/src/execution/shuffle/spark_unsafe/list.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/native/core/src/execution/shuffle/spark_unsafe/list.rs b/native/core/src/execution/shuffle/spark_unsafe/list.rs index 7cf5fa6b05..31e986ddbc 100644 --- a/native/core/src/execution/shuffle/spark_unsafe/list.rs +++ b/native/core/src/execution/shuffle/spark_unsafe/list.rs @@ -109,8 +109,9 @@ impl SparkUnsafeArray { // SAFETY: addr points to valid Spark UnsafeArray data from the JVM. // The first 8 bytes contain the element count as a little-endian i64. debug_assert!(addr != 0, "SparkUnsafeArray::new: null address"); - // SAFETY: Spark UnsafeArray stores element count as the first 8 bytes, 8-byte aligned. - let num_elements = unsafe { *(addr as *const i64) }; + // SAFETY: Spark UnsafeArray stores element count as the first 8 bytes. + // Use read_unaligned because nested arrays may not be 8-byte aligned. + let num_elements = unsafe { (addr as *const i64).read_unaligned() }; if num_elements < 0 { panic!("Negative number of elements: {num_elements}"); From 2e20b316f2a2517b100dd21fff6316ac90e20b9b Mon Sep 17 00:00:00 2001 From: Andy Grove Date: Wed, 11 Mar 2026 08:48:06 -0600 Subject: [PATCH 4/5] test: add regression tests for misaligned SparkUnsafeArray Verify that SparkUnsafeArray construction and element access work correctly at non-8-byte-aligned addresses, as occurs when arrays are nested within Spark UnsafeRow. --- .../execution/shuffle/spark_unsafe/list.rs | 90 +++++++++++++++++++ 1 file changed, 90 insertions(+) diff --git a/native/core/src/execution/shuffle/spark_unsafe/list.rs b/native/core/src/execution/shuffle/spark_unsafe/list.rs index 31e986ddbc..0a07e2d281 100644 --- a/native/core/src/execution/shuffle/spark_unsafe/list.rs +++ b/native/core/src/execution/shuffle/spark_unsafe/list.rs @@ -479,3 +479,93 @@ pub fn append_list_element( Ok(()) } + +#[cfg(test)] +mod test { + use super::*; + use arrow::array::builder::Int32Builder; + use arrow::array::Array; + + /// Helper to create a SparkUnsafeArray buffer with i32 elements. + /// Layout: 8 bytes num_elements + null bitset + element data. + fn create_i32_array_buffer(values: &[i32], null_indices: &[usize]) -> Vec { + let num_elements = values.len(); + let null_bitset_words = num_elements.div_ceil(64); + let header_size = 8 + null_bitset_words * 8; + let data_size = num_elements * 4; + let mut buffer = vec![0u8; header_size + data_size]; + + buffer[0..8].copy_from_slice(&(num_elements as i64).to_le_bytes()); + + for &idx in null_indices { + let word_offset = 8 + (idx / 64) * 8; + let current = + i64::from_le_bytes(buffer[word_offset..word_offset + 8].try_into().unwrap()); + let updated = current | (1i64 << (idx % 64)); + buffer[word_offset..word_offset + 8].copy_from_slice(&updated.to_le_bytes()); + } + + for (i, &val) in values.iter().enumerate() { + let offset = header_size + i * 4; + buffer[offset..offset + 4].copy_from_slice(&val.to_le_bytes()); + } + + buffer + } + + /// Test that SparkUnsafeArray works correctly when placed at a misaligned + /// address. This is a regression test for a bug where `SparkUnsafeArray::new` + /// used a direct pointer dereference `*(addr as *const i64)` which panics + /// on non-8-byte-aligned addresses. Nested arrays within Spark UnsafeRow + /// can be at arbitrary offsets. + #[test] + fn test_misaligned_array_construction() { + let values = vec![10i32, 20, 30, 40, 50]; + let buffer = create_i32_array_buffer(&values, &[]); + + // Place the array data at a 4-byte-aligned but not 8-byte-aligned offset + // by prepending 4 bytes. This simulates a nested array within a row + // where preceding fields cause misalignment. + let mut misaligned_buf = vec![0u8; 4 + buffer.len()]; + misaligned_buf[4..].copy_from_slice(&buffer); + let misaligned_addr = misaligned_buf.as_ptr() as usize + 4; + assert_ne!(misaligned_addr % 8, 0, "address should not be 8-byte aligned"); + + let array = SparkUnsafeArray::new(misaligned_addr as i64); + assert_eq!(array.get_num_elements(), 5); + + let mut builder = Int32Builder::with_capacity(5); + append_to_builder::(&DataType::Int32, &mut builder, &array).unwrap(); + let result = builder.finish(); + assert_eq!(result.len(), 5); + for (i, &expected) in values.iter().enumerate() { + assert_eq!(result.value(i), expected); + } + } + + /// Test misaligned array with nullable elements. + #[test] + fn test_misaligned_array_with_nulls() { + let values = vec![100i32, 0, 300, 0, 500]; + let null_indices = vec![1, 3]; + let buffer = create_i32_array_buffer(&values, &null_indices); + + let mut misaligned_buf = vec![0u8; 4 + buffer.len()]; + misaligned_buf[4..].copy_from_slice(&buffer); + let misaligned_addr = misaligned_buf.as_ptr() as usize + 4; + assert_ne!(misaligned_addr % 8, 0); + + let array = SparkUnsafeArray::new(misaligned_addr as i64); + assert_eq!(array.get_num_elements(), 5); + + let mut builder = Int32Builder::with_capacity(5); + append_to_builder::(&DataType::Int32, &mut builder, &array).unwrap(); + let result = builder.finish(); + assert_eq!(result.len(), 5); + assert_eq!(result.value(0), 100); + assert!(result.is_null(1)); + assert_eq!(result.value(2), 300); + assert!(result.is_null(3)); + assert_eq!(result.value(4), 500); + } +} From df735eeaa2b2687a23a7dc311e3b760ce145a692 Mon Sep 17 00:00:00 2001 From: Andy Grove Date: Wed, 11 Mar 2026 11:04:49 -0600 Subject: [PATCH 5/5] cargo fmt --- native/core/src/execution/shuffle/spark_unsafe/list.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/native/core/src/execution/shuffle/spark_unsafe/list.rs b/native/core/src/execution/shuffle/spark_unsafe/list.rs index 0a07e2d281..5549e0fb7a 100644 --- a/native/core/src/execution/shuffle/spark_unsafe/list.rs +++ b/native/core/src/execution/shuffle/spark_unsafe/list.rs @@ -529,7 +529,11 @@ mod test { let mut misaligned_buf = vec![0u8; 4 + buffer.len()]; misaligned_buf[4..].copy_from_slice(&buffer); let misaligned_addr = misaligned_buf.as_ptr() as usize + 4; - assert_ne!(misaligned_addr % 8, 0, "address should not be 8-byte aligned"); + assert_ne!( + misaligned_addr % 8, + 0, + "address should not be 8-byte aligned" + ); let array = SparkUnsafeArray::new(misaligned_addr as i64); assert_eq!(array.get_num_elements(), 5);