Add benchmarks for Utf8View scalars for zip

[mkleen]

Which issue does this PR close?

N/A

Rationale for this change

I have a PR to improve zip perf for Utf8View/BinaryView scalars and I need benchmarks for that.

• Add special implementation for zip for Utf8View/BinaryView scalars #8963

What changes are included in this PR?

This extends the zip benchmarks by one new Input Generator for StringViews and two more functions to test scalar combinations of different StringViews combinations.

Are these changes tested?

N/A

Are there any user-facing changes?

No

[alamb]

Thanks @mkleen

[alamb]

I don't understand this code:

1. It seems to be making an array (not a scalar)
2. generate_non_null_scalars returns a vector but then the callsite only ever seems to use a single return value -- why not just return a single array?

[mkleen]

Good point, generate_non_null_scalars is also used at https://github.com/apache/arrow-rs/blob/main/arrow/benches/zip_kernels.rs#L156-L157 to retrieve two values.

[mkleen]

Would you prefer to introduce a generate_non_null_scalar function returning a single Array for all InputGenerators ?

[mkleen]

I pushed a simplified version. I hope this makes more sense to you.

[alamb]

Thanks @mkleen

[alamb]

Sorry -- I still don't get this benchmark -- calling input_generator_1.generate() results in a array (not a scalar):

impl GenerateStringView {
    fn name(&self) -> &str {
        self.description.as_str()
    }
    fn generate_null(&self) -> ArrayRef {
        new_null_array(&DataType::Utf8View, 1)
    }

    fn generate(&self) -> ArrayRef {
        Arc::new(create_string_view_array_with_fixed_len(
            1,
            0.0,
            self.str_len,
        ))
    }
}

I would have expected a test something like

let null_scalar = input_generator.generate_null_scalar();
let non_null_scalar = input_generator.generate_scalar(); // make a 1 row array
let array = input_generator.generate_array(); // make a 8192 array

...
And then you could run the combinations 

(array, non_null_scalar),
(non_null_scalar, array),
(array, null_scalar),
(null_scalar, array)

[mkleen]

I intended to create the benchmark for the combination where both sides are scalars, with the following combinations:

(non_null_scalar, non_null_scalar)
(null_scalar, non_null_scalar)
(non_null_scalar, null_scalar)

and different string-view sizes on each side with different distributions of true/false etc. Each value is an array with a single value, which is then wrapped in a Scalar to match this condition:

https://github.com/apache/arrow-rs/blob/main/arrow-select/src/zip.rs#L104

and dispatch to the implementation to zip two scalars.

So you are saying this assumption is not correct and it should be instead (array, non_null_scalar) with an array of the length 8192 ?

Then i need to rework the benchmarks completely.

[mkleen]

I took this code from the given zip benchmark as foundation:

https://github.com/apache/arrow-rs/blob/main/arrow/benches/zip_kernels.rs#L149-L183

[mkleen]

I just double-checked the benchmarks from #8653 (the original optimization) see #8653 (comment). The optimisation only kicks in when both truthy and falsy are scalars, meaning arrays with single rows. The array vs scalar case is not improved.

[mkleen]

Ok, the generator returns now scalars instead of arrays directly. Regarding the combinations: I still think these are valid. The optimisation is based on scalar vs scalar values.

[mkleen]

Dear @alamb,

I just pushed another version. This just implements the existing InputGenerator trait for StringViews and uses the already existing benchmark function generating the following combinations:

(null_scalar, non_null_scalar)
(non_null_scalar,  null_scalar)
(non_nulls_scalar, non_nulls_scalar)
(array, non_null_scalar)
(non_null_scalar, array)
(array, array)

This does the job to show the improvements of #8963 for the first three combinations in the spirit of #8653

If you prefer to also refactor the benchmarks further with your suggestion regarding the return types of the InputGenerator, I am happy to do so, however this would mean I would need to adapt already existing benchmark code. Let me know.