Csaba Ringhofer has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/13807 )

Change subject: IMPALA-8741: Speed up bit unpacking by vectorisation
......................................................................


Patch Set 15:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/13807/15/be/src/util/vectorised_bit_unpacking_generator.py
File be/src/util/vectorised_bit_unpacking_generator.py:

http://gerrit.cloudera.org:8080/#/c/13807/15/be/src/util/vectorised_bit_unpacking_generator.py@5
PS15, Line 5: """
Can you extend this comment? The following info could be useful:
- this script doesn't need to be run during impala builds/test, so it can be 
python 3
- the general signature of the generated functions (maybe even a whole function 
could be added as an example, and an assert could be added to main as a smoke 
test to check whether it is generated for the given parameters)
- the main types of the different implementations (bit scatter, AVX2 4 values 
at a time, AVX2 8 values at a time) could be mentioned with some rough 
explanation, possibly links to the most important instructions. The reasons why 
so many different algorithms were needed could be also mentioned, e.g. that 
vector operations are both constrained during read (how many values fit to a 
32/64 bit broadcast read) and unpacking (there is only 32/64 bit variable 
length shifting, so there is no way to handle more than 8 values at a time)


http://gerrit.cloudera.org:8080/#/c/13807/15/be/src/util/vectorised_bit_unpacking_generator.py@39
PS15, Line 39:
style: we generally use indentation of 2 for block in Impala, even in py code



--
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: 15
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: Wed, 24 Jul 2019 17:32:05 +0000
Gerrit-HasComments: Yes

Reply via email to