Jim Apple has posted comments on this change. Change subject: Use AVX2 operations to speedup Bloom filters by 10-100%. ......................................................................
Patch Set 6: (5 comments) I have changed the layout of the BFs so that they are the same no matter what instruction set is supported in the writer. There are some performance changes from this, but they are mostly a wash. http://gerrit.cloudera.org:8080/#/c/3338/5/be/src/util/bloom-filter.h File be/src/util/bloom-filter.h: PS5, Line 158: > typo: constants Done Line 187: const uint32_t bucket_idx = HashUtil::Rehash32to32(hash) & directory_mask_; > If the datalayout is impacted, then I agree having a member makes sense. T I have tried to clarify this in the comments and I have changed the code to use the same layout whether AVX2 instructions are enabled or not. PS5, Line 188: (CpuInfo::IsSup > can you double check that we aren't double dispatching? I.e. is this a dir I have checked that we are not double-dispatching. However, we are not inlining the AVX functions inside the functions without target("avx2"). I will fix this in a later patch by following the strategy in sse-util.h. Line 246: > Is there some reason we need to zero them here but not in the Find() implem No; that was simply an omission Line 258: > The logic to compute the mask is identical to above, right? Can we make it Done -- 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: 6 Gerrit-Project: Impala Gerrit-Branch: cdh5-trunk Gerrit-Owner: Jim Apple <[email protected]> Gerrit-Reviewer: Dan Hecht <[email protected]> Gerrit-Reviewer: Jim Apple <[email protected]> Gerrit-Reviewer: Tim Armstrong <[email protected]> Gerrit-HasComments: Yes
