Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Optimize MutableArrayData::extend for null buffers #397

Closed
jhorstmann opened this issue Jun 4, 2021 · 2 comments · Fixed by #716
Closed

Optimize MutableArrayData::extend for null buffers #397

jhorstmann opened this issue Jun 4, 2021 · 2 comments · Fixed by #716
Assignees
Labels
enhancement Any new improvement worthy of a entry in the changelog

Comments

@jhorstmann
Copy link
Contributor

jhorstmann commented Jun 4, 2021

In one of our benchmarks the concat kernel was identified as a big performance bottleneck while sorting, specifically the closures inside build_extend_null_bits which calls set_bits. The logic in there currently sets individual bits and also contains a branch for every bit

if bit_util::get_bit(...) {
    bit_util::set_bit(...);
}

I think it should be possible to rewrite this to set multiple bits at the same time and remove most of the branch overhead. The general idea would look like this:

  • append individual bits until the destination buffer starts at a byte offset
  • start a BitChunk iterator on the source buffer and then append u8 or u64 at a time
  • append the remainder u8 at a time

Similar logic would apply to setting all bits to valid, appending chunks of u8::MAX or u64::MAX at a time.

The get_bit / set_bit functions themselves could probably also be speed up a little, I think on modern processors calculating the bit masks instead of using a lookup table should be faster. But after the above changes, those functions would no longer be used in the hot path.

@jhorstmann jhorstmann added the enhancement Any new improvement worthy of a entry in the changelog label Jun 4, 2021
@mathiaspeters-sig
Copy link
Contributor

Since I can't assign myself I'll comment instead: I'm starting to work on this now

@Dandandan
Copy link
Contributor

@mathiaspeters-sig

There might be some inspiration to use from the work in arrow2 by @jorgecarleitao , e.g. see jorgecarleitao/arrow2#291

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Any new improvement worthy of a entry in the changelog
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants