Daniel Becker has posted comments on this change. ( http://gerrit.cloudera.org:8080/13807 )
Change subject: IMPALA-8741: Speed up bit unpacking by vectorisation ...................................................................... Patch Set 22: (3 comments) http://gerrit.cloudera.org:8080/#/c/13807/22/be/src/benchmarks/bit-packing-benchmark.cc File be/src/benchmarks/bit-packing-benchmark.cc: http://gerrit.cloudera.org:8080/#/c/13807/22/be/src/benchmarks/bit-packing-benchmark.cc@39 PS22, Line 39: // BitReader 9.85e+03 9.98e+03 1.01e+04 1X 1X 1X Updated the benchmarks. http://gerrit.cloudera.org:8080/#/c/13807/22/be/src/util/bit-packing.inline.h File be/src/util/bit-packing.inline.h: http://gerrit.cloudera.org:8080/#/c/13807/22/be/src/util/bit-packing.inline.h@284 PS22, Line 284: // TODO: Decide if this function is needed. It seems that it is only used in the I think we could remove this method. It is only used in the old benchmark where we compare the original BitReader that unpacks a single value at a time, this method and UnpackValues that can unpack any number of values (not just 32). Many times the last one is faster than Unpack32Values which is not very intuitive so maybe there is some problem with the benchmark, too. So I think we could either remove this function completely, also from the benchmarks, or if we need it there, we could move it to the benchmark file instead of BitPacking. http://gerrit.cloudera.org:8080/#/c/13807/22/be/src/util/vectorised_bit_unpacking_generator.py File be/src/util/vectorised_bit_unpacking_generator.py: http://gerrit.cloudera.org:8080/#/c/13807/22/be/src/util/vectorised_bit_unpacking_generator.py@1281 PS22, Line 1281: elif bit_width in range(5, 9): Here, with bit width 6, BitScatter is a little bit faster according to my benchmark results. We could branch here in and use BitScatter in that case but that 1-2% is probably not worth it. -- To view, visit http://gerrit.cloudera.org:8080/13807 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I9e452a547973778bbd8d768c608e1a32e948f947 Gerrit-Change-Number: 13807 Gerrit-PatchSet: 22 Gerrit-Owner: Daniel Becker <daniel.bec...@cloudera.com> Gerrit-Reviewer: Csaba Ringhofer <csringho...@cloudera.com> Gerrit-Reviewer: Daniel Becker <daniel.bec...@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Tim Armstrong <tarmstr...@cloudera.com> Gerrit-Comment-Date: Mon, 29 Jul 2019 11:27:36 +0000 Gerrit-HasComments: Yes
