XiangpengHao commented on code in PR #5871:
URL: https://github.com/apache/arrow-rs/pull/5871#discussion_r1635419387
##########
arrow-cast/src/cast/dictionary.rs:
##########
@@ -85,10 +85,97 @@ pub(crate) fn dictionary_cast<K: ArrowDictionaryKeyType>(
Ok(new_array)
}
+ Utf8View => {
+ // `unpack_dictionary` can handle Utf8View/BinaryView types, but
incurs unnecessary data copy of the value buffer.
+ // we handle it here to avoid the copy.
+ let dict_array = array
+ .as_dictionary::<K>()
+ .downcast_dict::<StringArray>()
+ .unwrap();
+
+ let string_values = dict_array.values();
+ let value_offsets = string_values.value_offsets();
+ let value_buffer = string_values.values().clone();
+
+ let view_buffer =
+ view_from_dict_values(value_offsets, &value_buffer,
dict_array.keys());
+
+ // Safety:
+ // the buffer is from StringArray which is utf8.
+ let string_view = unsafe {
+ StringViewArray::new_unchecked(
+ view_buffer,
+ vec![value_buffer],
+ dict_array.nulls().cloned(),
+ )
+ };
+ Ok(Arc::new(string_view))
+ }
+ BinaryView => {
+ // `unpack_dictionary` can handle Utf8View/BinaryView types, but
incurs unnecessary data copy of the value buffer.
+ // we handle it here to avoid the copy.
+ let dict_array = array
+ .as_dictionary::<K>()
+ .downcast_dict::<BinaryArray>()
+ .unwrap();
+
+ let binary_values = dict_array.values();
+ let value_offsets = binary_values.value_offsets();
+ let value_buffer = binary_values.values().clone();
+
+ let view_buffer =
+ view_from_dict_values(value_offsets, &value_buffer,
dict_array.keys());
+ let binary_view = unsafe {
+ BinaryViewArray::new_unchecked(
+ view_buffer,
+ vec![value_buffer],
+ dict_array.nulls().cloned(),
+ )
+ };
+ Ok(Arc::new(binary_view))
+ }
_ => unpack_dictionary::<K>(array, to_type, cast_options),
}
}
+fn view_from_dict_values<K: ArrowDictionaryKeyType>(
+ value_offsets: &[i32],
+ value_buffer: &arrow_buffer::Buffer,
+ keys: &PrimitiveArray<K>,
+) -> ScalarBuffer<u128> {
+ let mut view_builder = BufferBuilder::<u128>::new(keys.len());
+ for i in keys.iter() {
+ match i {
+ Some(v) => {
+ let idx = v.to_usize().unwrap();
+ let offset = value_offsets[idx];
+ let end = value_offsets[idx + 1];
+ let length = end - offset;
+ let value_buf = &value_buffer[offset as usize..end as usize];
+
+ if length <= 12 {
Review Comment:
Unfortunately, adding `append_view_unchecked` improved the performance by 50%
```
dict to view time: [38.116 µs 38.122 µs 38.127 µs]
change: [-50.618% -50.455% -50.290%] (p = 0.00 <
0.05)
Performance has improved.
Found 8 outliers among 100 measurements (8.00%)
1 (1.00%) low severe
4 (4.00%) low mild
1 (1.00%) high mild
2 (2.00%) high severe
```
I think it justifies adding `append_view_unchecked`, what do you think?
Also, should I add the benchmark to the repo? It's a bit tricky to setup two
versions of the cast implementation..
--
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]
