Youwei Wang has posted comments on this change. Change subject: IMPALA-2809: Improve ByteSwap with builtin function or SSE or AVX2. ......................................................................
Patch Set 15: (2 comments) http://gerrit.cloudera.org:8080/#/c/3081/15/be/src/benchmarks/bswap-benchmark.cc File be/src/benchmarks/bswap-benchmark.cc: > yes - each component function should be tested, too. According to our discussion, these component functions: ByteSwapSSE_Unit/ByteSwapAVX2_Unit may be overthrown since we are going to redefine our logic here. Please refer the next comment post in this batch. Line 124: void TestSSE42Swap(int batch_size, void* d) { > I am having trouble understand what you are proposing. Can you try to summa Hi Jim. I am afraid SIMD approach has limited effects if applied inside the function BitUtil::ByteSwap. The reason is according to its caller, the maximum data width this function deals with is 128bit (the Decimal16Value), which only satisfies the minimum requirement of SSE4.2. If we still want this SIMD optimization approach, I am afraid we should move to the callers of BitUtil::ByteSwap for only there exists some chance we can see a large DecimalXvalue array instead of one single DecimalXvalue. In summary, I am afraid our previous approach is not too much useful if used inside the BitUtil::ByteSwap. Also it is not the thing the original patch intends to do. Actually, we should find out the caller of BitUtil::ByteSwap, remove the BitUtil::ByteSwap code, and substitute it with our SIMD approach if such caller is dealing with a large array of DecimalXvalue. Sorry for confusing you with previous ambiguous description. If you still find this description unclear, please don’t hesitate to tell me. Thank you. -- To view, visit http://gerrit.cloudera.org:8080/3081 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I392ed5a8d5683f30f161282c228c1aedd7b648c1 Gerrit-PatchSet: 15 Gerrit-Project: Impala Gerrit-Branch: cdh5-trunk Gerrit-Owner: Youwei Wang <[email protected]> Gerrit-Reviewer: Alex Behm <[email protected]> Gerrit-Reviewer: Jim Apple <[email protected]> Gerrit-Reviewer: Marcel Kornacker <[email protected]> Gerrit-Reviewer: Mostafa Mokhtar <[email protected]> Gerrit-Reviewer: Tim Armstrong <[email protected]> Gerrit-Reviewer: Youwei Wang <[email protected]> Gerrit-HasComments: Yes
