[Java] Optimize bit operations performance

[asfimport]

From @animeshtrivedi's benchmark finding:

2. Materialize values from Validity and Value direct buffers instead of
   calling getInt() function on the IntVector. This is implemented as a new
   Unsafe reader type (
   https://github.com/animeshtrivedi/benchmarking-arrow/blob/master/src/main/java/com/github/animeshtrivedi/benchmark/ArrowReaderUnsafe.java#L31
   )

3. Optimize bitmap operation to check if a bit is set or not (
   https://github.com/animeshtrivedi/benchmarking-arrow/blob/master/src/main/java/com/github/animeshtrivedi/benchmark/ArrowReaderUnsafe.java#L23
   )

Reporter: Li Jin / @icexelloss

Subtasks:

• [Java] Remove Long.bitCount with simple bitmap operations

PRs and other links:

• GitHub Pull Request #5930

_{Note: This issue was originally created as ARROW-3495. Please see the migration documentation for further details.}

[asfimport]

Animesh Trivedi / @animeshtrivedi:
This one has 3 items that need to be worked separately 

1. delete the optimized bitmap (for all null or all set case) routine in BitVectorHelper (https://github.com/apache/arrow/blob/master/java/vector/src/main/java/org/apache/arrow/vector/BitVectorHelper.java#L179) - i am not sure what is the sense behind this optimization. As I wrote on the mailing list, at this point the validity buffer is already read from the storage, no need to spend more CPU time to generate an optimized bitmap. 

 

2. change the implementation of isSet function in (UnionVector.java , BaseFixedWidthVector.java ,  BaseVariableWidthVector.java , FixedSizeListVector.java , ListVector.java , StructVector.java) to test for equality with zero. No need to count number of bits set in a Long as done here : https://github.com/apache/arrow/blob/master/java/vector/src/main/java/org/apache/arrow/vector/BaseFixedWidthVector.java#L797

 

3. I need to think about the UnsafeReader type, and how it should be integrated. 

 

I can open a pull request for the first two items. 

[asfimport]

Wes McKinney / @wesm:
Is there still interest in this?

[asfimport]

Animesh Trivedi / @animeshtrivedi:
Hi @wesm, thanks for catching up. I have been on an extended break since beginning of the year, but I am starting to catch up my pending items now. I will push the bitmap optimizations in coming weeks. Hope that is ok. 

[asfimport]

Wes McKinney / @wesm:
OK, sounds good!

[asfimport]

Azim Afroozeh:
Hi all,

I'm opening the following pull request for this issue: #5930

This patch does the following changes:

• moves two common function "getNullCount" and "splitAndTransferValidityBuffer" to the top-level BaseValueVector. This change requires moving "validityBuffer" to the BaseValueVector class (as recommended in this TODO: https://github.com/apache/arrow/blob/master/java/vector/src/main/java/org/apache/arrow/vector/BaseFixedWidthVector.java#L89)

• optimize the implementation of loadValidityBuffer (in the BaseValueVector) to just pass the reference for the validity buffer instead of optimizing it

• optimize for the common boundary condition when all variables are valid (as done in the C++ code: https://github.com/apache/arrow/blob/master/cpp/src/arrow/array.h#L290)

  The optimization delivers performance.

  Tests: Read 50M integers from a single Int column (2GB).

  Before the patch:
  Baseline: 7.64 Gb/sec
  With the Holder API: 9.99 Gb/sec

  After the patch (with the bitmap condition checks)
  Baseline: 12.13 Gb/sec (+58.7% gains)
  With the Holder API: 16.03 Gb/sec (+60.4% gains)

[asfimport]

Todd Farmer / @toddfarmer:
This issue was last updated over 90 days ago, which may be an indication it is no longer being actively worked. To better reflect the current state, the issue is being unassigned. Please feel free to re-take assignment of the issue if it is being actively worked, or if you plan to start that work soon.