prtkgaur opened a new pull request, #51250:
URL: https://github.com/apache/arrow/pull/51250

   ### Rationale for this change
   
   The miniblocks of a DELTA_BINARY_PACKED block are packed back to back with 
no padding between
   them, so a run of miniblocks that share a bit width is bit-identical to a 
single longer run at
   that width. `GetInternal` calls the bit unpacker once per miniblock all the 
same, which with
   the default geometry is one call per 32 values -- at narrow widths, mostly 
per-call setup.
   
   ### What changes are included in this PR?
   
   A look-ahead over the block's stored bit widths, in a small private helper, 
reports how many of
   the following miniblocks may be folded into the current unpack call, so a 
run of four asks the
   unpacker for 128 values instead of 32. A miniblock joins the run only when 
its stored width
   *equals* the current `delta_bit_width_`, which `InitMiniBlock` has already 
validated, and the
   run also stops at the end of the block and at what the caller has room for.
   
   ### Are these changes tested?
   
   A new typed test covers the width patterns that decide where a run starts 
and stops, and the
   fixture's read batch sizes gain one that stops partway through a coalesced 
run, so the existing
   round trips cover it too. Three mutations of the implementation -- ignoring 
the neighbour's
   width, ignoring the caller's room, and failing to advance the block cursor 
-- each turn the new
   test red.
   
   ### Benchmark
   
   Same setup as the previous PR: AWS Graviton4, GCC 11.5, `Release`, one core, 
9 repetitions,
   medians, 65,536 values, both arms built and measured in one sitting. The two 
`Fixed` arms are
   the `delta_bit_width_ == 0` path, which has no unpack call to coalesce.
   
   | benchmark | previous commit | this PR | | vs. main |
   |---|--:|--:|--:|--:|
   | `Decode_Int32_Narrow` | 78.11 us | 58.61 us | **1.33x** | **1.70x** |
   | `Decode_Int32_Wide` | 80.79 us | 65.08 us | **1.24x** | **1.57x** |
   | `Decode_Int64_Narrow` | 64.41 us | 55.08 us | **1.17x** | **1.49x** |
   | `Decode_Int64_Wide` | 308.65 us | 301.60 us | 1.02x | 1.06x |
   | `Decode_Int32_Fixed` | 19.75 us | 20.31 us | 0.97x | 0.97x |
   | `Decode_Int64_Fixed` | 32.20 us | 31.74 us | 1.01x | 0.97x |
   
   ### Are there any user-facing changes?
   
   No. No API change, no format change, and decoded values are identical.
   
   **This PR includes breaking changes to public APIs.** (If there are any 
breaking changes to public APIs, please explain which changes are breaking. If 
not, you can remove this.)
   
   **This PR contains a "Critical Fix".** (If the changes fix either (a) a 
security vulnerability, (b) a bug that caused incorrect or invalid data to be 
produced, or (c) a bug that causes a crash (even when the API contract is 
upheld), please provide explanation. If not, you can remove this.)
   


-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to