Re: [PR] GH-44513: [C++] Fix overflow issues for large build side in swiss join [arrow]

["pitrou (via GitHub)" <gi...@apache.org>]

pitrou commented on code in PR #45108:
URL: https://github.com/apache/arrow/pull/45108#discussion_r1904313094


##########
cpp/src/arrow/compute/key_map_internal.h:
##########
@@ -214,6 +214,14 @@ class ARROW_EXPORT SwissTable {
   int log_minibatch_;
   // Base 2 log of the number of blocks
   int log_blocks_ = 0;
+  // When log_blocks_ is greater than 25, there will be overlapping bits between block id
+  // and stamp within a 32-bit hash value. So we must check if this is the case when
+  // right-shifting a hash value to retrieve block id and stamp. The following two
+  // variables will be derived from log_blocks_ as log_blocks changes, and use them as the
+  // number of bits to right shift, rather than branching on whether log_blocks_ > 25
+  // every time in tight loops.
+  int bits_shift_for_block_and_stamp_ = bits_hash_ - log_blocks_ - bits_stamp_;
+  int bits_shift_for_block_ = bits_stamp_;

Review Comment:
   Since the computation is repeated several times, perhaps we can have a short helper function to factor it out? Something like:
   ```c++
     static std::pair<int, int> ComputeBitShifts(int log_blocks) {
       if (log_blocks + bits_stamp_ > bits_hash_) {
         return {0, bits_hash_ - log_blocks};
       } else {
         return {bits_hash_ - log_blocks - bits_stamp_, bits_stamp_};
       }
     }
   ```
   



-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org