You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@doris.apache.org by "Mryange (via GitHub)" <gi...@apache.org> on 2023/04/07 04:20:12 UTC

[GitHub] [doris] Mryange opened a new pull request, #18456: [refactor](planner) In the runtime filter, use crc_hash replace original murmur_hash

Mryange opened a new pull request, #18456:
URL: https://github.com/apache/doris/pull/18456

   # Proposed changes
   
   After testing, crchash is 4 to 5 times faster than murmurhash.
   
   ## Problem summary
   
   When be_exec_version is 1, the original murmurhash method is still used. If updated, the new crchash method will be used.
   
   Describe your changes.
   
   
   ## Checklist(Required)
   
   * [ ] Does it affect the original behavior
   * [ ] Has unit tests been added
   * [ ] Has document been added or modified
   * [ ] Does it need to update dependencies
   * [ ] Is this PR support rollback (If NO, please explain WHY)
   
   ## Further comments
   
   If this is a relatively large or complex change, kick off the discussion at [dev@doris.apache.org](mailto:dev@doris.apache.org) by explaining why you chose the solution you did and what alternatives you considered, etc...
   
   


-- 
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: commits-unsubscribe@doris.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org


[GitHub] [doris] Mryange commented on pull request #18456: [refactor](planner) In the runtime filter, use crc_hash replace original murmur_hash

Posted by "Mryange (via GitHub)" <gi...@apache.org>.
Mryange commented on PR #18456:
URL: https://github.com/apache/doris/pull/18456#issuecomment-1499943067

   run buildall


-- 
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: commits-unsubscribe@doris.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org


[GitHub] [doris] github-actions[bot] commented on a diff in pull request #18456: [refactor](planner) In the runtime filter, use crc_hash replace original murmur_hash

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on code in PR #18456:
URL: https://github.com/apache/doris/pull/18456#discussion_r1160426006


##########
be/src/exprs/block_bloom_filter.hpp:
##########
@@ -78,6 +80,49 @@ class BlockBloomFilter {
         }
     }
 
+    // This function is only to be used if the be_exec_version may be less than 2. If updated, please delete it.
+    void insert_new_hash(const Slice& key) noexcept {
+        if (key.data) {
+            insert(HashUtil::crc_hash(key.data, key.size, _hash_seed));
+        }
+    }
+
+#ifdef __AVX2__
+
+    static inline ATTRIBUTE_ALWAYS_INLINE __attribute__((__target__("avx2"))) __m256i make_mark(
+            const uint32_t hash) {
+        const __m256i ones = _mm256_set1_epi32(1);
+        const __m256i rehash = _mm256_setr_epi32(BLOOM_HASH_CONSTANTS);
+        // Load hash into a YMM register, repeated eight times
+        __m256i hash_data = _mm256_set1_epi32(hash);
+        // Multiply-shift hashing ala Dietzfelbinger et al.: multiply 'hash' by eight different
+        // odd constants, then keep the 5 most significant bits from each product.
+        hash_data = _mm256_mullo_epi32(rehash, hash_data);
+        hash_data = _mm256_srli_epi32(hash_data, 27);
+        // Use these 5 bits to shift a single bit to a location in each 32-bit lane
+        return _mm256_sllv_epi32(ones, hash_data);
+    }
+#endif
+
+
+
+#ifdef __AVX2__
+
+    static inline ATTRIBUTE_ALWAYS_INLINE __attribute__((__target__("avx2"))) __m256i make_mark(

Review Comment:
   warning: class member cannot be redeclared [clang-diagnostic-error]
   ```cpp
       static inline ATTRIBUTE_ALWAYS_INLINE __attribute__((__target__("avx2"))) __m256i make_mark(
                                                                                         ^
   ```
   **be/src/exprs/block_bloom_filter.hpp:91:** previous definition is here
   ```cpp
       static inline ATTRIBUTE_ALWAYS_INLINE __attribute__((__target__("avx2"))) __m256i make_mark(
                                                                                         ^
   ```
   



##########
be/src/exprs/block_bloom_filter.hpp:
##########
@@ -113,6 +158,25 @@
         return result;
 #else
         return bucket_find(bucket_idx, hash);
+#endif
+    }
+    ALWAYS_INLINE bool find(uint32_t hash) const noexcept {

Review Comment:
   warning: class member cannot be redeclared [clang-diagnostic-error]
   ```cpp
       ALWAYS_INLINE bool find(uint32_t hash) const noexcept {
                          ^
   ```
   **be/src/exprs/block_bloom_filter.hpp:143:** previous definition is here
   ```cpp
       ALWAYS_INLINE bool find(uint32_t hash) const noexcept {
                          ^
   ```
   



##########
be/src/exprs/block_bloom_filter.hpp:
##########
@@ -78,6 +80,49 @@
         }
     }
 
+    // This function is only to be used if the be_exec_version may be less than 2. If updated, please delete it.
+    void insert_new_hash(const Slice& key) noexcept {
+        if (key.data) {
+            insert(HashUtil::crc_hash(key.data, key.size, _hash_seed));
+        }
+    }
+
+#ifdef __AVX2__
+
+    static inline ATTRIBUTE_ALWAYS_INLINE __attribute__((__target__("avx2"))) __m256i make_mark(
+            const uint32_t hash) {
+        const __m256i ones = _mm256_set1_epi32(1);
+        const __m256i rehash = _mm256_setr_epi32(BLOOM_HASH_CONSTANTS);
+        // Load hash into a YMM register, repeated eight times
+        __m256i hash_data = _mm256_set1_epi32(hash);
+        // Multiply-shift hashing ala Dietzfelbinger et al.: multiply 'hash' by eight different
+        // odd constants, then keep the 5 most significant bits from each product.
+        hash_data = _mm256_mullo_epi32(rehash, hash_data);
+        hash_data = _mm256_srli_epi32(hash_data, 27);
+        // Use these 5 bits to shift a single bit to a location in each 32-bit lane
+        return _mm256_sllv_epi32(ones, hash_data);
+    }
+#endif
+
+
+
+#ifdef __AVX2__
+
+    static inline ATTRIBUTE_ALWAYS_INLINE __attribute__((__target__("avx2"))) __m256i make_mark(
+            const uint32_t hash) {
+        const __m256i ones = _mm256_set1_epi32(1);
+        const __m256i rehash = _mm256_setr_epi32(BLOOM_HASH_CONSTANTS);
+        // Load hash into a YMM register, repeated eight times
+        __m256i hash_data = _mm256_set1_epi32(hash);
+        // Multiply-shift hashing ala Dietzfelbinger et al.: multiply 'hash' by eight different
+        // odd constants, then keep the 5 most significant bits from each product.
+        hash_data = _mm256_mullo_epi32(rehash, hash_data);
+        hash_data = _mm256_srli_epi32(hash_data, 27);
+        // Use these 5 bits to shift a single bit to a location in each 32-bit lane
+        return _mm256_sllv_epi32(ones, hash_data);
+    }
+#endif
+
 #ifdef __AVX2__
 
     static inline ATTRIBUTE_ALWAYS_INLINE __attribute__((__target__("avx2"))) __m256i make_mark(

Review Comment:
   warning: class member cannot be redeclared [clang-diagnostic-error]
   ```cpp
       static inline ATTRIBUTE_ALWAYS_INLINE __attribute__((__target__("avx2"))) __m256i make_mark(
                                                                                         ^
   ```
   **be/src/exprs/block_bloom_filter.hpp:91:** previous definition is here
   ```cpp
       static inline ATTRIBUTE_ALWAYS_INLINE __attribute__((__target__("avx2"))) __m256i make_mark(
                                                                                         ^
   ```
   



-- 
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: commits-unsubscribe@doris.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org


[GitHub] [doris] Mryange closed pull request #18456: [refactor](planner) In the runtime filter, use crc_hash replace original murmur_hash

Posted by "Mryange (via GitHub)" <gi...@apache.org>.
Mryange closed pull request #18456:  [refactor](planner) In the runtime filter, use crc_hash replace original murmur_hash
URL: https://github.com/apache/doris/pull/18456


-- 
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: commits-unsubscribe@doris.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org