Jim Apple has posted comments on this change.

Change subject: Use AVX2 operations to speedup Bloom filter insert by 10-50%.
......................................................................


Patch Set 2:

(5 comments)

http://gerrit.cloudera.org:8080/#/c/3338/2/be/src/util/bloom-filter-test.cc
File be/src/util/bloom-filter-test.cc:

Line 48: TEST(BloomFilter, Insert) {
> Can we add some test coverage with AVX2 disabled?
Done


http://gerrit.cloudera.org:8080/#/c/3338/2/be/src/util/bloom-filter.h
File be/src/util/bloom-filter.h:

Line 129:   // A faster SIMD version of Insert()
> /// instead of //
Done


Line 154:     const uint32_t bucket_idx = HashUtil::Rehash32to32(hash) & 
directory_mask_;
> Factor this out into InsertGeneric()
Called them Insert, InsertBucket, InsertBucketAVX2. Not married to the names.


PS2, Line 184: reinterpret_cast<__m256i*>(
> This cast violates C's strict aliasing rules since we end up accessing the 
Done


http://gerrit.cloudera.org:8080/#/c/3338/2/bin/impala-config.sh
File bin/impala-config.sh:

Line 395:   LD_LIBRARY_PATH="${IMPALA_TOOLCHAIN_GCC_LIB}:${LD_LIBRARY_PATH}"
> This seems fine, but it may be worth running a private impala packaging bui
Packaging build failed with no useful error message.

    00:15:24.447 make[1]: *** [be/src/service/CMakeFiles/impalad.dir/rule] 
Error 2

I'll work on fixing it, but everything else is ready for re-review.


-- 
To view, visit http://gerrit.cloudera.org:8080/3338
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I6fef4f6652876f8fd7e3f0e41431702380418c98
Gerrit-PatchSet: 2
Gerrit-Project: Impala
Gerrit-Branch: cdh5-trunk
Gerrit-Owner: Jim Apple <[email protected]>
Gerrit-Reviewer: Jim Apple <[email protected]>
Gerrit-Reviewer: Tim Armstrong <[email protected]>
Gerrit-HasComments: Yes

Reply via email to