Optimize memory footprint of view arrays from ScalarValue::to_array_of_size #19441
Conversation
| let mut builder = | ||
| StringViewBuilder::with_capacity(size).with_deduplicate_strings(); | ||
| for _ in 0..size { | ||
| builder.append_value(value); | ||
| } | ||
| let array = builder.finish(); | ||
| Arc::new(array) |
There was a problem hiding this comment.
Technically we could further optimize this by manually calling StringViewArray::try_new with correct data and avoid need to hash as part of the builder; felt that might get too into the weeds of arrow-rs code, so stuck with this simpler approach
There was a problem hiding this comment.
Actually if we have a benchmark maybe it could be worth testing out if there is a performance gain by doing this? 🤔
| let buffers = array.data_buffers(); | ||
| assert_eq!(1, buffers.len()); | ||
| // Ensure we only have a single copy of the value string | ||
| assert_eq!(value.len(), buffers[0].len()); |
There was a problem hiding this comment.
On main this assert fails as the data buffer would be 10 * value.len(); with this fix we only have a single copy of the full string in the child buffer, minimizing memory footprint of the output array
| Arc::new(StringViewArray::from_iter_values(repeat_n(value, size))) | ||
| let mut builder = | ||
| StringViewBuilder::with_capacity(size).with_deduplicate_strings(); | ||
| for _ in 0..size { |
There was a problem hiding this comment.
🤔 we should prob have some kind of append_n to remove this boilerplate, in some future for arrow-rs
2010YOUY01 left a comment
2010YOUY01 left a comment There was a problem hiding this comment.
It's a good idea to keep it simple now. I agree if we can add a new API in arrow-rs to let it directly construct StringView arrays with repeating element of size k, it's likely to be much faster. (should we open an issue?)
| An |
4 participants
Which issue does this PR close?
ScalarValue::to_array_of_size#19440Rationale for this change
When we have view scalars (utf8/binary) and we call
to_array_of_size, the data buffers the resultant arrays have contains duplicate data. This is because the APIs we use don't deduplicate the data, instead appending it each time even though the data is exactly duplicated.What changes are included in this PR?
Manually use a builder with deduplication enabled.
Are these changes tested?
Added test.
Are there any user-facing changes?
No.