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 10: (4 comments) http://gerrit.cloudera.org:8080/#/c/13807/4/be/src/util/bit-packing-vectorized.h File be/src/util/bit-packing-vectorized.h: PS4: > Ahh I see. I don't think it should be too bad, we have similar CMake rules, I'll discuss it with other team members also. The generated file now includes a comment with the exact command line that was used to generate it. http://gerrit.cloudera.org:8080/#/c/13807/8/be/src/util/vectorised_bit_unpacking_generator.py File be/src/util/vectorised_bit_unpacking_generator.py: http://gerrit.cloudera.org:8080/#/c/13807/8/be/src/util/vectorised_bit_unpacking_generator.py@1540 PS8, Line 1540: > I'm included to remove this option and pick one alternative as the canonica I've removed the option and we always use clang-format. Using clang-format, the generated file conforms to the coding standard automatically and we don't have to take it into account in the code generating script. http://gerrit.cloudera.org:8080/#/c/13807/8/be/src/util/vectorised_bit_unpacking_generator.py@1544 PS8, Line 1544: stdout=subprocess.PIPE) > If we're going with clang-format, I think we should write the non-formatted I used pipes to avoid overwriting the file and using temporary files. http://gerrit.cloudera.org:8080/#/c/13807/8/be/src/util/vectorised_bit_unpacking_generator.py@1547 PS8, Line 1547: output_file.write(clang_format_process_result.stdout) > I think the script should just fail if clang_format was enabled and couldn' The script fails if it cannot find clang-format. -- 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: 10 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, 22 Jul 2019 09:07:33 +0000 Gerrit-HasComments: Yes
