tyrelr commented on a change in pull request #9215:
URL: https://github.com/apache/arrow/pull/9215#discussion_r562406187
##########
File path: rust/arrow/src/array/array_primitive.rs
##########
@@ -86,13 +86,9 @@ impl<T: ArrowPrimitiveType> PrimitiveArray<T> {
}
/// Returns the primitive value at index `i`.
- ///
- /// Note this doesn't do any bound checking, for performance reason.
- /// # Safety
- /// caller must ensure that the passed in offset is less than the array
len()
+ #[inline]
pub fn value(&self, i: usize) -> T::Native {
- let offset = i + self.offset();
- unsafe { *self.raw_values.as_ptr().add(offset) }
+ self.values()[i]
Review comment:
That makes sense. I'll rebase dropping that commit.
##########
File path: rust/arrow/src/array/array_primitive.rs
##########
@@ -86,13 +86,9 @@ impl<T: ArrowPrimitiveType> PrimitiveArray<T> {
}
/// Returns the primitive value at index `i`.
- ///
- /// Note this doesn't do any bound checking, for performance reason.
- /// # Safety
- /// caller must ensure that the passed in offset is less than the array
len()
+ #[inline]
pub fn value(&self, i: usize) -> T::Native {
- let offset = i + self.offset();
- unsafe { *self.raw_values.as_ptr().add(offset) }
+ self.values()[i]
Review comment:
safety overhead on a few benchmarks seems to be 15-30%
```
group master-
safe-prim- unsafe-prim-
----- -------
---------- ------------
add 512 1.00 307.4±4.79ns ? B/sec 1.07
327.7±6.48ns ? B/sec 1.30 400.5±7.09ns ? B/sec
add_nulls_512 1.20 447.8±8.09ns ? B/sec 1.32
490.9±3.41ns ? B/sec 1.00 372.2±5.96ns ? B/sec
buffer_bit_ops or 1.17 731.0±20.02ns ? B/sec 1.19
743.7±5.43ns ? B/sec 1.00 624.0±10.86ns ? B/sec
cast date64 to date32 512 1.00 7.4±0.22µs ? B/sec 1.10
8.2±0.07µs ? B/sec 1.03 7.7±0.22µs ? B/sec
cast i64 to string 512 1.00 24.8±0.33µs ? B/sec 1.09
27.1±0.79µs ? B/sec 1.10 27.3±0.41µs ? B/sec
cast time32s to time32ms 512 1.00 917.0±19.20ns ? B/sec 1.21
1113.0±22.04ns ? B/sec 1.07 984.5±7.53ns ? B/sec
equal_512 1.00 44.1±0.81ns ? B/sec 1.13
49.9±0.65ns ? B/sec 1.01 44.4±0.27ns ? B/sec
min nulls string 512 1.04 6.4±0.12µs ? B/sec 1.14
7.0±0.15µs ? B/sec 1.00 6.2±0.09µs ? B/sec
multiply 512 1.14 471.5±10.15ns ? B/sec 1.18
487.0±4.72ns ? B/sec 1.00 413.4±6.16ns ? B/sec
not 1.00 1102.3±14.55ns ? B/sec 1.12
1235.9±19.91ns ? B/sec 1.02 1120.3±19.83ns ? B/sec
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
