klion26 commented on PR #7987:
URL: https://github.com/apache/arrow-rs/pull/7987#issuecomment-3118389103
@scovich thanks for the detailed review and suggestion.
> Do the benchmarks cover different offset sizes, is_large true/false, etc?
Or are they always the same offset size?
The benchmark contains vary lenght of lists, but all the lenght less than
255 -- `is_large` is always false
I've run the following benchmarks
1. previous `1` with clone-free
2. Fifth approach
3. Sixth approach
The results show that the `sixth`is better. I've updated the implementation
to the sixth approach.
> The steps to generate the result:
> 1. Change the previous `1` approach to clone-free
> 2. Code fifth approach
> 3. Code sixth approach
> 4. Execute the command `cargo bench
--features=arrow,async,test_common,experimental --bench variant_kernels --
--save-baseline $BRANCH_NAME` for the three approaches and the main branch, one
by one.
> 5. execute the command `critcmp main ${BRANCH_NAME} ` for the three
approaches.
## pervious `1` with clone-free
```
group
7977_avoid_extra_buffer_with_packedu32_iterator main
-----
----------------------------------------------- ----
batch_json_string_to_variant json_list 8k string 1.00
33.5±1.17ms ? ?/sec 1.10 36.8±1.49ms ? ?/sec
batch_json_string_to_variant random_json(2633 bytes per document) 1.00
338.9±7.71ms ? ?/sec 1.10 373.5±8.07ms ? ?/sec
batch_json_string_to_variant repeated_struct 8k string 1.02
13.6±0.45ms ? ?/sec 1.00 13.3±0.47ms ? ?/sec
variant_get_primitive 1.00
2.1±0.07ms ? ?/sec 1.01 2.1±0.07ms ? ?/sec
```
## fifth approach
```
group
7977_pre_populate_with_push_exten main
-----
--------------------------------- ----
batch_json_string_to_variant json_list 8k string 1.00
35.8±1.52ms ? ?/sec 1.03 36.8±1.49ms ? ?/sec
batch_json_string_to_variant random_json(2633 bytes per document) 1.00
342.9±9.65ms ? ?/sec 1.09 373.5±8.07ms ? ?/sec
batch_json_string_to_variant repeated_struct 8k string 1.00
13.1±0.46ms ? ?/sec 1.01 13.3±0.47ms ? ?/sec
variant_get_primitive 1.00
2.1±0.07ms ? ?/sec 1.00 2.1±0.07ms ? ?/sec
```
<details>
<summary>code for fifth approach</summary>
let header = array_header(is_large, offset_size);
let mut bytes_to_splice = vec![header];
let num_elements_bytes =
num_elements
.to_le_bytes()
.into_iter()
.take(if is_large { 4 } else { 1 });
bytes_to_splice.extend(num_elements_bytes);
let offsets = PackedU32Iterator::new(
offset_size as usize,
self.offsets
.iter()
.map(|&offset| (offset as u32).to_le_bytes()),
);
bytes_to_splice.extend(offsets);
let data_size_bytes = data_size
.to_le_bytes()
.into_iter()
.take(offset_size as usize);
bytes_to_splice.extend(data_size_bytes);
buffer
.inner_mut()
.splice(starting_offset..starting_offset, bytes_to_splice);
</details>
## sixth approach
```
group
7977_pre_populate_with_directly_append_bytes main
-----
-------------------------------------------- ----
batch_json_string_to_variant json_list 8k string 1.00
33.7±1.21ms ? ?/sec 1.09 36.8±1.49ms ? ?/sec
batch_json_string_to_variant random_json(2633 bytes per document) 1.00
333.4±7.89ms ? ?/sec 1.12 373.5±8.07ms ? ?/sec
batch_json_string_to_variant repeated_struct 8k string 1.00
13.2±0.46ms ? ?/sec 1.01 13.3±0.47ms ? ?/sec
variant_get_primitive 1.00
2.1±0.08ms ? ?/sec 1.00 2.1±0.07ms ? ?/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.
To unsubscribe, e-mail: github-unsubscr...@arrow.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
