Jefffrey commented on code in PR #10668:
URL: https://github.com/apache/arrow-rs/pull/10668#discussion_r3789500198
##########
arrow-cast/src/cast/decimal.rs:
##########
@@ -652,40 +722,35 @@ where
.and_then(|v| T::is_valid_decimal_precision(v,
precision).then_some(v))
});
// Benefit:
- // 20% performance improvement
+ // 15-19% faster than appending to a PrimitiveBuilder (measured
+ // with the cast_kernels string-to-decimal benchmarks)
// Soundness:
- // The iterator is trustedLen because it comes from an
`StringArray`.
+ // The iterator is trustedLen because it comes from a
`StringArray`.
Ok(unsafe {
PrimitiveArray::<T>::from_trusted_len_iter(iter)
.with_precision_and_scale(precision, scale)?
})
} else {
- let vec = from
- .iter()
- .map(|v| {
- v.map(|v| {
- parse_string_to_decimal_native::<T>(v, scale as usize)
- .map_err(|_| {
+ let mut builder = PrimitiveBuilder::<T>::with_capacity(from.len());
Review Comment:
note: this change isnt captured in cast_kernels benchmark since we only
benchmark for `safe = true`
##########
arrow-cast/src/cast/decimal.rs:
##########
@@ -539,100 +549,160 @@ where
T::Native: DecimalCast + ArrowNativeTypeOp,
{
let value_str = value_str.trim();
- let parts: Vec<&str> = value_str.split('.').collect();
- if parts.len() > 2 {
- return Err(ArrowError::InvalidArgumentError(format!(
- "Invalid decimal format: {value_str:?}"
- )));
- }
+ let bytes = value_str.as_bytes();
- let (negative, first_part) = if parts[0].is_empty() {
- (false, parts[0])
- } else {
- match parts[0].as_bytes()[0] {
- b'-' => (true, &parts[0][1..]),
- b'+' => (false, &parts[0][1..]),
- _ => (false, parts[0]),
+ let mut index = 0;
+ let negative = match bytes.first() {
+ Some(b'-') => {
+ index += 1;
+ true
+ }
+ Some(b'+') => {
+ index += 1;
+ false
}
+ _ => false,
};
- let integers = first_part;
- let decimals = if parts.len() == 2 { parts[1] } else { "" };
+ let mut value = T::Native::ZERO;
+ let mut chunk = 0_u64;
+ let mut chunk_len = 0_usize;
+ let mut saw_digit = false;
+ let mut saw_point = false;
+ let mut fractionals = 0_usize;
+ let mut first_discarded_digit = None;
+
+ while let Some(&b) = bytes.get(index) {
+ match b {
+ b'0'..=b'9' => {
+ saw_digit = true;
+ let digit = b - b'0';
+ if saw_point {
+ if fractionals == scale {
+ first_discarded_digit.get_or_insert(digit);
+ index += 1;
+ continue;
Review Comment:
i do wonder if instead of continuing via the loop to verify we have a valid
number, we specialize to another loop that checks if the remainder characters
is an ascii digit; it might be able to vectorize better?
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
