Re: [C++] Reducing branching in compute/kernels/vector_selection.cc

[Niranda Perera <ni...@gmail.com>]

I have created a PR for the changes we discussed.
https://github.com/apache/arrow/pull/10679

It would be great if you guys could go through it. I'm still benchmarking
the results. And I also have some ideas to reduce the branches in the main
while loop in the Primitive Types implementation. I will try to add that as
well.

Best

On Tue, Jun 29, 2021 at 12:10 PM Niranda Perera <ni...@gmail.com>
wrote:

> Ah. Sorry, my mistake! there's a separate `ExecNonNull()` method! 😇
>
> On Tue, Jun 29, 2021 at 12:00 PM Antoine Pitrou <an...@python.org>
> wrote:
>
>>
>> Le 29/06/2021 à 17:58, Niranda Perera a écrit :
>> > So, FWIU, in vector selection, the output array would always have a
>> > non-null validity buffer, isn't it?
>>
>> Why?
>>
>>
>> >
>> > On Tue, Jun 29, 2021 at 11:54 AM Antoine Pitrou <an...@python.org>
>> wrote:
>> >
>> >>
>> >>
>> >> Le 29/06/2021 à 17:49, Niranda Perera a écrit :
>> >>> Hi all,
>> >>>
>> >>> I'm looking into this now, and I feel like there's a potential memory
>> >>> corruption at the very end of the out_data_ array.
>> >>>
>> >>> algo:
>> >>> bool advance = BitUtil::GetBit(filter_data_, filter_offset_ +
>> >> in_position);
>> >>> BitUtil::SetBitTo(out_is_valid_, out_offset_ + out_position_,
>> advance);
>> >>> out_data_[out_position_] = values_data_[in_position];
>> >>> out_position_ += advance; // may need static_cast<int> here
>> >>> ++in_position;
>> >>> say,
>> >>> in.data =   [1, 2, 3, 4] // all valid
>> >>> filt.data = [1, 1, 0, 0] // all valid
>> >>> At in_position = 1, out_position becomes 2. And till the end of the
>> loop
>> >>> (i.e. in_position <= 3), out_position[2] will be updated (but with
>> >>> out_is_valid_[2] set to 0). I belive at the end of the loop, following
>> >>> would be the output.
>> >>> out.data =  [1, 2, 4]
>> >>> out.valid = [1, 1, 0]
>> >>>
>> >>> Wouldn't this be a problem?
>> >>
>> >> Indeed, you have to allocate N + 1 entries for the result array instead
>> >> of N.  Probably not a big deal, IMHO.
>> >>
>> >> Regards
>> >>
>> >> Antoine.
>> >>
>> >
>> >
>>
>
>
> --
> Niranda Perera
> https://niranda.dev/
> @n1r44 <https://twitter.com/N1R44>
>
>

-- 
Niranda Perera
https://niranda.dev/
@n1r44 <https://twitter.com/N1R44>