[Impala-ASF-CR] IMPALA-8741: Speed up bit unpacking by vectorisation

2019-08-07 Thread Daniel Becker (Code Review)
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 23:

We found that the improvement in UnpackValues doesn't help much with dictionary 
and delta decoding, because dict decoding uses UnpackAndDecodeValues, which 
decodes the value right after unpacking as opposed to first unpacking all 
values and then decoding them. Delta is also faster this way.

Therefore it is uncertain if introducing vectorisation is worth the added 
complexity now.

On the other hand, restricting the unpacking output types to unsigned integers 
is independent from vectorisation and is a good idea, so I'll create a separate 
issue for 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: 23
Gerrit-Owner: Daniel Becker 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Daniel Becker 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Wed, 07 Aug 2019 15:58:22 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-8741: Speed up bit unpacking by vectorisation

2019-07-30 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/13807 )

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


Patch Set 23:

Build Successful

https://jenkins.impala.io/job/gerrit-code-review-checks/4079/ : Initial code 
review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun 
to run full precommit tests.


--
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: 23
Gerrit-Owner: Daniel Becker 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Daniel Becker 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Tue, 30 Jul 2019 15:04:23 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-8741: Speed up bit unpacking by vectorisation

2019-07-30 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/13807 )

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


Patch Set 23:

(3 comments)

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

http://gerrit.cloudera.org:8080/#/c/13807/23/be/src/util/vectorised_bit_unpacking_generator.py@93
PS23, Line 93: from typing import List, Optional, Tuple
flake8: F401 'typing.Tuple' imported but unused


http://gerrit.cloudera.org:8080/#/c/13807/23/be/src/util/vectorised_bit_unpacking_generator.py@93
PS23, Line 93: from typing import List, Optional, Tuple
flake8: F401 'typing.Optional' imported but unused


http://gerrit.cloudera.org:8080/#/c/13807/23/be/src/util/vectorised_bit_unpacking_generator.py@93
PS23, Line 93: from typing import List, Optional, Tuple
flake8: F401 'typing.List' imported but unused



--
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: 23
Gerrit-Owner: Daniel Becker 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Daniel Becker 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Tue, 30 Jul 2019 14:23:40 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-8741: Speed up bit unpacking by vectorisation

2019-07-30 Thread Daniel Becker (Code Review)
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 23:

(1 comment)

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:
> I think we could remove this method. It is only used in the old benchmark w
I removed it and updated the benchmark results.



--
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: 23
Gerrit-Owner: Daniel Becker 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Daniel Becker 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Tue, 30 Jul 2019 14:23:12 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-8741: Speed up bit unpacking by vectorisation

2019-07-30 Thread Daniel Becker (Code Review)
Daniel Becker has uploaded a new patch set (#23). ( 
http://gerrit.cloudera.org:8080/13807 )

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

IMPALA-8741: Speed up bit unpacking by vectorisation

Adding a vectorised implementation to bit unpacking using AVX, AVX2 and
BMI2 instructions through compiler intrinsics.

Vectorised bit unpacking is implemented for bit widths from 1 to 16.
Higher bit widths would not benefit from this vectorisation algorithm.

We check at runtime whether the required instructions are available on
the CPU and fall back to the scalar implementation if not.

The vectorised unpacking functions are in the file
be/src/util/bit-packing-vectorized.h, which is generated by the python
script in be/src/util/vectorised_bit_unpacking_generator.py.

Also adding benchmarks comparing the scalar and the vectorised
implementations.

Testing:
  - Added tests for the vectorised unpacking implementations.

Change-Id: I9e452a547973778bbd8d768c608e1a32e948f947
---
M be/src/benchmarks/bit-packing-benchmark.cc
M be/src/exec/parquet/parquet-bool-decoder.h
M be/src/util/bit-packing-test.cc
A be/src/util/bit-packing-vectorized.generated.h
M be/src/util/bit-packing.cc
M be/src/util/bit-packing.h
M be/src/util/bit-packing.inline.h
M be/src/util/bit-stream-utils-test.cc
M be/src/util/bit-stream-utils.h
M be/src/util/cpu-info.cc
M be/src/util/cpu-info.h
A be/src/util/vectorised_bit_unpacking_generator.py
12 files changed, 6,246 insertions(+), 230 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/07/13807/23
--
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: newpatchset
Gerrit-Change-Id: I9e452a547973778bbd8d768c608e1a32e948f947
Gerrit-Change-Number: 13807
Gerrit-PatchSet: 23
Gerrit-Owner: Daniel Becker 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Daniel Becker 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 


[Impala-ASF-CR] IMPALA-8741: Speed up bit unpacking by vectorisation

2019-07-29 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins 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:

Build Successful

https://jenkins.impala.io/job/gerrit-code-review-checks/4060/ : Initial code 
review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun 
to run full precommit tests.


--
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 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Daniel Becker 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Mon, 29 Jul 2019 12:08:49 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-8741: Speed up bit unpacking by vectorisation

2019-07-29 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins 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/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@93
PS22, Line 93: from typing import List, Optional, Tuple
flake8: F401 'typing.Tuple' imported but unused


http://gerrit.cloudera.org:8080/#/c/13807/22/be/src/util/vectorised_bit_unpacking_generator.py@93
PS22, Line 93: from typing import List, Optional, Tuple
flake8: F401 'typing.Optional' imported but unused


http://gerrit.cloudera.org:8080/#/c/13807/22/be/src/util/vectorised_bit_unpacking_generator.py@93
PS22, Line 93: from typing import List, Optional, Tuple
flake8: F401 'typing.List' imported but unused



--
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 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Daniel Becker 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Mon, 29 Jul 2019 11:28:13 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-8741: Speed up bit unpacking by vectorisation

2019-07-29 Thread Daniel Becker (Code Review)
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 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Daniel Becker 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Mon, 29 Jul 2019 11:27:36 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-8741: Speed up bit unpacking by vectorisation

2019-07-29 Thread Daniel Becker (Code Review)
Daniel Becker has uploaded a new patch set (#22). ( 
http://gerrit.cloudera.org:8080/13807 )

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

IMPALA-8741: Speed up bit unpacking by vectorisation

Adding a vectorised implementation to bit unpacking using AVX, AVX2 and
BMI2 instructions through compiler intrinsics.

Vectorised bit unpacking is implemented for bit widths from 1 to 16.
Higher bit widths would not benefit from this vectorisation algorithm.

We check at runtime whether the required instructions are available on
the CPU and fall back to the scalar implementation if not.

The vectorised unpacking functions are in the file
be/src/util/bit-packing-vectorized.h, which is generated by the python
script in be/src/util/vectorised_bit_unpacking_generator.py.

Also adding benchmarks comparing the scalar and the vectorised
implementations.

Testing:
  - Added tests for the vectorised unpacking implementations.

Change-Id: I9e452a547973778bbd8d768c608e1a32e948f947
---
M be/src/benchmarks/bit-packing-benchmark.cc
M be/src/exec/parquet/parquet-bool-decoder.h
M be/src/util/bit-packing-test.cc
A be/src/util/bit-packing-vectorized.generated.h
M be/src/util/bit-packing.cc
M be/src/util/bit-packing.h
M be/src/util/bit-packing.inline.h
M be/src/util/bit-stream-utils-test.cc
M be/src/util/bit-stream-utils.h
M be/src/util/cpu-info.cc
M be/src/util/cpu-info.h
A be/src/util/vectorised_bit_unpacking_generator.py
12 files changed, 6,293 insertions(+), 198 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/07/13807/22
--
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: newpatchset
Gerrit-Change-Id: I9e452a547973778bbd8d768c608e1a32e948f947
Gerrit-Change-Number: 13807
Gerrit-PatchSet: 22
Gerrit-Owner: Daniel Becker 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Daniel Becker 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 


[Impala-ASF-CR] IMPALA-8741: Speed up bit unpacking by vectorisation

2019-07-29 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/13807 )

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


Patch Set 21:

Build Successful

https://jenkins.impala.io/job/gerrit-code-review-checks/4059/ : Initial code 
review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun 
to run full precommit tests.


--
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: 21
Gerrit-Owner: Daniel Becker 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Daniel Becker 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Mon, 29 Jul 2019 08:44:26 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-8741: Speed up bit unpacking by vectorisation

2019-07-29 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/13807 )

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


Patch Set 21:

(3 comments)

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

http://gerrit.cloudera.org:8080/#/c/13807/21/be/src/util/vectorised_bit_unpacking_generator.py@93
PS21, Line 93: from typing import List, Optional, Tuple
flake8: F401 'typing.Tuple' imported but unused


http://gerrit.cloudera.org:8080/#/c/13807/21/be/src/util/vectorised_bit_unpacking_generator.py@93
PS21, Line 93: from typing import List, Optional, Tuple
flake8: F401 'typing.Optional' imported but unused


http://gerrit.cloudera.org:8080/#/c/13807/21/be/src/util/vectorised_bit_unpacking_generator.py@93
PS21, Line 93: from typing import List, Optional, Tuple
flake8: F401 'typing.List' imported but unused



--
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: 21
Gerrit-Owner: Daniel Becker 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Daniel Becker 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Mon, 29 Jul 2019 08:04:29 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-8741: Speed up bit unpacking by vectorisation

2019-07-29 Thread Daniel Becker (Code Review)
Daniel Becker has uploaded a new patch set (#21). ( 
http://gerrit.cloudera.org:8080/13807 )

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

IMPALA-8741: Speed up bit unpacking by vectorisation

Adding a vectorised implementation to bit unpacking using AVX, AVX2 and
BMI2 instructions through compiler intrinsics.

Vectorised bit unpacking is implemented for bit widths from 1 to 16.
Higher bit widths would not benefit from this vectorisation algorithm.

We check at runtime whether the required instructions are available on
the CPU and fall back to the scalar implementation if not.

The vectorised unpacking functions are in the file
be/src/util/bit-packing-vectorized.h, which is generated by the python
script in be/src/util/vectorised_bit_unpacking_generator.py.

Also adding benchmarks comparing the scalar and the vectorised
implementations.

Testing:
  - Added tests for the vectorised unpacking implementations.

Change-Id: I9e452a547973778bbd8d768c608e1a32e948f947
---
M be/src/benchmarks/bit-packing-benchmark.cc
M be/src/exec/parquet/parquet-bool-decoder.h
M be/src/util/bit-packing-test.cc
A be/src/util/bit-packing-vectorized.generated.h
M be/src/util/bit-packing.cc
M be/src/util/bit-packing.h
M be/src/util/bit-packing.inline.h
M be/src/util/bit-stream-utils-test.cc
M be/src/util/bit-stream-utils.h
M be/src/util/cpu-info.cc
M be/src/util/cpu-info.h
A be/src/util/vectorised_bit_unpacking_generator.py
12 files changed, 6,194 insertions(+), 99 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/07/13807/21
--
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: newpatchset
Gerrit-Change-Id: I9e452a547973778bbd8d768c608e1a32e948f947
Gerrit-Change-Number: 13807
Gerrit-PatchSet: 21
Gerrit-Owner: Daniel Becker 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Daniel Becker 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 


[Impala-ASF-CR] IMPALA-8741: Speed up bit unpacking by vectorisation

2019-07-29 Thread Daniel Becker (Code Review)
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 21:

(2 comments)

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

http://gerrit.cloudera.org:8080/#/c/13807/19/be/src/util/vectorised_bit_unpacking_generator.py@76
PS19, Line 76:
> Removed metaclass inheritance, so I'm going to remove this.
Accidentally I found a way to use abstract base classes without the metaclass 
syntax, I should have just looked at the abc module documentation. I simply 
derive the class from ABC instead of using ABCMeta as a metaclass. This way 
flake8 has no problem with it.


http://gerrit.cloudera.org:8080/#/c/13807/19/be/src/util/vectorised_bit_unpacking_generator.py@89
PS19, Line 89:
> Yeah we can ignore this, it will only get re-flagged if someone touches the
I added a comment describing why we have those imports.



--
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: 21
Gerrit-Owner: Daniel Becker 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Daniel Becker 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Mon, 29 Jul 2019 08:03:22 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-8741: Speed up bit unpacking by vectorisation

2019-07-26 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/13807 )

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


Patch Set 19:

(1 comment)

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

http://gerrit.cloudera.org:8080/#/c/13807/19/be/src/util/vectorised_bit_unpacking_generator.py@89
PS19, Line 89: from typing import List, Optional, Tuple
> The unused imports are used by mypy, a static type checker I like to use. I
Yeah we can ignore this, it will only get re-flagged if someone touches these 
lines in a new patchset. There's a reason these are just comments rather than a 
hard failure - there are false positives. If you want to tweak 
bin/jenkins/critique-gerrit-review.py to ignore this special case, feel free.



--
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: 19
Gerrit-Owner: Daniel Becker 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Daniel Becker 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Fri, 26 Jul 2019 19:12:05 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-8741: Speed up bit unpacking by vectorisation

2019-07-26 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/13807 )

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


Patch Set 20:

Build Successful

https://jenkins.impala.io/job/gerrit-code-review-checks/4028/ : Initial code 
review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun 
to run full precommit tests.


--
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: 20
Gerrit-Owner: Daniel Becker 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Daniel Becker 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Fri, 26 Jul 2019 19:09:06 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-8741: Speed up bit unpacking by vectorisation

2019-07-26 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/13807 )

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


Patch Set 19:

Build Successful

https://jenkins.impala.io/job/gerrit-code-review-checks/4027/ : Initial code 
review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun 
to run full precommit tests.


--
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: 19
Gerrit-Owner: Daniel Becker 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Daniel Becker 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Fri, 26 Jul 2019 18:52:46 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-8741: Speed up bit unpacking by vectorisation

2019-07-26 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/13807 )

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


Patch Set 20:

(4 comments)

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

http://gerrit.cloudera.org:8080/#/c/13807/20/be/src/util/vectorised_bit_unpacking_generator.py@76
PS20, Line 76: from abc import ABCMeta, abstractmethod, abstractproperty
flake8: F401 'abc.ABCMeta' imported but unused


http://gerrit.cloudera.org:8080/#/c/13807/20/be/src/util/vectorised_bit_unpacking_generator.py@89
PS20, Line 89: from typing import List, Optional, Tuple
flake8: F401 'typing.Tuple' imported but unused


http://gerrit.cloudera.org:8080/#/c/13807/20/be/src/util/vectorised_bit_unpacking_generator.py@89
PS20, Line 89: from typing import List, Optional, Tuple
flake8: F401 'typing.Optional' imported but unused


http://gerrit.cloudera.org:8080/#/c/13807/20/be/src/util/vectorised_bit_unpacking_generator.py@89
PS20, Line 89: from typing import List, Optional, Tuple
flake8: F401 'typing.List' imported but unused



--
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: 20
Gerrit-Owner: Daniel Becker 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Daniel Becker 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Fri, 26 Jul 2019 18:48:31 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-8741: Speed up bit unpacking by vectorisation

2019-07-26 Thread Daniel Becker (Code Review)
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 20:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/13807/18/be/src/util/bit-packing.inline.h
File be/src/util/bit-packing.inline.h:

http://gerrit.cloudera.org:8080/#/c/13807/18/be/src/util/bit-packing.inline.h@268
PS18, Line 268: DCHECK_GE(in_bytes, BYTES_TO_READ_IN_BATCH * 
batches_to_read);
It should be DCHECK_GE(in_bytes, BYTES_TO_READ_IN_BATCH * batches_to_read).


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

http://gerrit.cloudera.org:8080/#/c/13807/19/be/src/util/vectorised_bit_unpacking_generator.py@76
PS19, Line 76: from abc import ABCMeta, abstractmethod, abstractproperty
> flake8: F401 'abc.ABCMeta' imported but unused
Removed metaclass inheritance, so I'm going to remove this.


http://gerrit.cloudera.org:8080/#/c/13807/19/be/src/util/vectorised_bit_unpacking_generator.py@89
PS19, Line 89: from typing import List, Optional, Tuple
> flake8: F401 'typing.Tuple' imported but unused
The unused imports are used by mypy, a static type checker I like to use. If 
this is a problem I can remove the imports and add them when I'd like to use 
mypy, but if they can stay it's better for me.



--
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: 20
Gerrit-Owner: Daniel Becker 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Daniel Becker 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Fri, 26 Jul 2019 18:48:50 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-8741: Speed up bit unpacking by vectorisation

2019-07-26 Thread Daniel Becker (Code Review)
Daniel Becker has uploaded a new patch set (#20). ( 
http://gerrit.cloudera.org:8080/13807 )

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

IMPALA-8741: Speed up bit unpacking by vectorisation

Adding a vectorised implementation to bit unpacking using AVX, AVX2 and
BMI2 instructions through compiler intrinsics.

Vectorised bit unpacking is implemented for bit widths from 1 to 16.
Higher bit widths would not benefit from this vectorisation algorithm.

We check at runtime whether the required instructions are available on
the CPU and fall back to the scalar implementation if not.

The vectorised unpacking functions are in the file
be/src/util/bit-packing-vectorized.h, which is generated by the python
script in be/src/util/vectorised_bit_unpacking_generator.py.

Also adding benchmarks comparing the scalar and the vectorised
implementations.

Testing:
  - Added tests for the vectorised unpacking implementations.

Change-Id: I9e452a547973778bbd8d768c608e1a32e948f947
---
M be/src/benchmarks/bit-packing-benchmark.cc
M be/src/exec/parquet/parquet-bool-decoder.h
M be/src/util/bit-packing-test.cc
A be/src/util/bit-packing-vectorized.generated.h
M be/src/util/bit-packing.cc
M be/src/util/bit-packing.h
M be/src/util/bit-packing.inline.h
M be/src/util/bit-stream-utils-test.cc
M be/src/util/bit-stream-utils.h
M be/src/util/cpu-info.cc
M be/src/util/cpu-info.h
A be/src/util/vectorised_bit_unpacking_generator.py
12 files changed, 6,189 insertions(+), 99 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/07/13807/20
--
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: newpatchset
Gerrit-Change-Id: I9e452a547973778bbd8d768c608e1a32e948f947
Gerrit-Change-Number: 13807
Gerrit-PatchSet: 20
Gerrit-Owner: Daniel Becker 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Daniel Becker 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 


[Impala-ASF-CR] IMPALA-8741: Speed up bit unpacking by vectorisation

2019-07-26 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/13807 )

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


Patch Set 19:

(4 comments)

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

http://gerrit.cloudera.org:8080/#/c/13807/19/be/src/util/vectorised_bit_unpacking_generator.py@76
PS19, Line 76: from abc import ABCMeta, abstractmethod, abstractproperty
flake8: F401 'abc.ABCMeta' imported but unused


http://gerrit.cloudera.org:8080/#/c/13807/19/be/src/util/vectorised_bit_unpacking_generator.py@89
PS19, Line 89: from typing import List, Optional, Tuple
flake8: F401 'typing.Tuple' imported but unused


http://gerrit.cloudera.org:8080/#/c/13807/19/be/src/util/vectorised_bit_unpacking_generator.py@89
PS19, Line 89: from typing import List, Optional, Tuple
flake8: F401 'typing.Optional' imported but unused


http://gerrit.cloudera.org:8080/#/c/13807/19/be/src/util/vectorised_bit_unpacking_generator.py@89
PS19, Line 89: from typing import List, Optional, Tuple
flake8: F401 'typing.List' imported but unused



--
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: 19
Gerrit-Owner: Daniel Becker 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Daniel Becker 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Fri, 26 Jul 2019 18:11:38 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-8741: Speed up bit unpacking by vectorisation

2019-07-26 Thread Daniel Becker (Code Review)
Daniel Becker has uploaded a new patch set (#19). ( 
http://gerrit.cloudera.org:8080/13807 )

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

IMPALA-8741: Speed up bit unpacking by vectorisation

Adding a vectorised implementation to bit unpacking using AVX, AVX2 and
BMI2 instructions through compiler intrinsics.

Vectorised bit unpacking is implemented for bit widths from 1 to 16.
Higher bit widths would not benefit from this vectorisation algorithm.

We check at runtime whether the required instructions are available on
the CPU and fall back to the scalar implementation if not.

The vectorised unpacking functions are in the file
be/src/util/bit-packing-vectorized.h, which is generated by the python
script in be/src/util/vectorised_bit_unpacking_generator.py.

Also adding benchmarks comparing the scalar and the vectorised
implementations.

Testing:
  - Added tests for the vectorised unpacking implementations.

Change-Id: I9e452a547973778bbd8d768c608e1a32e948f947
---
M be/src/benchmarks/bit-packing-benchmark.cc
M be/src/exec/parquet/parquet-bool-decoder.h
M be/src/util/bit-packing-test.cc
A be/src/util/bit-packing-vectorized.generated.h
M be/src/util/bit-packing.cc
M be/src/util/bit-packing.h
M be/src/util/bit-packing.inline.h
M be/src/util/bit-stream-utils-test.cc
M be/src/util/bit-stream-utils.h
M be/src/util/cpu-info.cc
M be/src/util/cpu-info.h
A be/src/util/vectorised_bit_unpacking_generator.py
12 files changed, 6,188 insertions(+), 98 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/07/13807/19
--
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: newpatchset
Gerrit-Change-Id: I9e452a547973778bbd8d768c608e1a32e948f947
Gerrit-Change-Number: 13807
Gerrit-PatchSet: 19
Gerrit-Owner: Daniel Becker 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Daniel Becker 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 


[Impala-ASF-CR] IMPALA-8741: Speed up bit unpacking by vectorisation

2019-07-26 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/13807 )

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


Patch Set 18:

Build Failed

https://jenkins.impala.io/job/gerrit-code-review-checks/4021/ : Initial code 
review checks failed. See linked job for details on the failure.


--
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: 18
Gerrit-Owner: Daniel Becker 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Daniel Becker 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Fri, 26 Jul 2019 17:26:59 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-8741: Speed up bit unpacking by vectorisation

2019-07-26 Thread Daniel Becker (Code Review)
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 18:

The segfault seems to have been caused by an alignment issue. _mm256_load_si256 
requires that the argument be aligned to 32 bytes. I added alignas(32) to the 
variables that are loaded this way in the code generator.


--
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: 18
Gerrit-Owner: Daniel Becker 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Daniel Becker 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Fri, 26 Jul 2019 16:48:00 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-8741: Speed up bit unpacking by vectorisation

2019-07-26 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/13807 )

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


Patch Set 18:

(2 comments)

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

http://gerrit.cloudera.org:8080/#/c/13807/18/be/src/util/vectorised_bit_unpacking_generator.py@259
PS18, Line 259: =
flake8: E999 SyntaxError: invalid syntax


http://gerrit.cloudera.org:8080/#/c/13807/18/be/src/util/vectorised_bit_unpacking_generator.py@588
PS18, Line 588: l
flake8: E501 line too long (93 > 90 characters)



--
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: 18
Gerrit-Owner: Daniel Becker 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Daniel Becker 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Fri, 26 Jul 2019 16:46:19 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-8741: Speed up bit unpacking by vectorisation

2019-07-26 Thread Daniel Becker (Code Review)
Daniel Becker has uploaded a new patch set (#18). ( 
http://gerrit.cloudera.org:8080/13807 )

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

IMPALA-8741: Speed up bit unpacking by vectorisation

Adding a vectorised implementation to bit unpacking using AVX, AVX2 and
BMI2 instructions through compiler intrinsics.

Vectorised bit unpacking is implemented for bit widths from 1 to 16.
Higher bit widths would not benefit from this vectorisation algorithm.

We check at runtime whether the required instructions are available on
the CPU and fall back to the scalar implementation if not.

The vectorised unpacking functions are in the file
be/src/util/bit-packing-vectorized.h, which is generated by the python
script in be/src/util/vectorised_bit_unpacking_generator.py.

Also adding benchmarks comparing the scalar and the vectorised
implementations.

Testing:
  - Added tests for the vectorised unpacking implementations.

Change-Id: I9e452a547973778bbd8d768c608e1a32e948f947
---
M be/src/benchmarks/bit-packing-benchmark.cc
M be/src/exec/parquet/parquet-bool-decoder.h
M be/src/util/bit-packing-test.cc
A be/src/util/bit-packing-vectorized.generated.h
M be/src/util/bit-packing.cc
M be/src/util/bit-packing.h
M be/src/util/bit-packing.inline.h
M be/src/util/bit-stream-utils-test.cc
M be/src/util/bit-stream-utils.h
M be/src/util/cpu-info.cc
M be/src/util/cpu-info.h
A be/src/util/vectorised_bit_unpacking_generator.py
12 files changed, 6,187 insertions(+), 98 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/07/13807/18
--
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: newpatchset
Gerrit-Change-Id: I9e452a547973778bbd8d768c608e1a32e948f947
Gerrit-Change-Number: 13807
Gerrit-PatchSet: 18
Gerrit-Owner: Daniel Becker 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Daniel Becker 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 


[Impala-ASF-CR] IMPALA-8741: Speed up bit unpacking by vectorisation

2019-07-26 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/13807 )

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


Patch Set 17:

Build Failed

https://jenkins.impala.io/job/gerrit-code-review-checks/4019/ : Initial code 
review checks failed. See linked job for details on the failure.


--
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: 17
Gerrit-Owner: Daniel Becker 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Daniel Becker 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Fri, 26 Jul 2019 16:32:45 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-8741: Speed up bit unpacking by vectorisation

2019-07-26 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/13807 )

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


Patch Set 17:

(1 comment)

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

http://gerrit.cloudera.org:8080/#/c/13807/17/be/src/util/vectorised_bit_unpacking_generator.py@259
PS17, Line 259: =
flake8: E999 SyntaxError: invalid syntax



--
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: 17
Gerrit-Owner: Daniel Becker 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Daniel Becker 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Fri, 26 Jul 2019 15:52:22 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-8741: Speed up bit unpacking by vectorisation

2019-07-26 Thread Daniel Becker (Code Review)
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 17:

(2 comments)

Please do not merge it yet, I have some ubsan tests segfaulting.

http://gerrit.cloudera.org:8080/#/c/13807/4/be/src/util/bit-packing-vectorized.h
File be/src/util/bit-packing-vectorized.h:

PS4:
> Yes, I prefer committing the generated file in this case. I can imagine a s
I added "generated" to the file name.


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:
Added the required comments.

About the mini smoke test: I think including just one example function has 
limited usefulness for testing purposes. Another difficulty is that we run the 
generated source file through clang-format, so comparing the generated raw 
function with the example taken from the file may be more difficult as they are 
not the same string, so we would need to run clang-format on the generated 
function, too.



--
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: 17
Gerrit-Owner: Daniel Becker 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Daniel Becker 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Fri, 26 Jul 2019 15:51:44 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-8741: Speed up bit unpacking by vectorisation

2019-07-26 Thread Daniel Becker (Code Review)
Daniel Becker has uploaded a new patch set (#17). ( 
http://gerrit.cloudera.org:8080/13807 )

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

IMPALA-8741: Speed up bit unpacking by vectorisation

Adding a vectorised implementation to bit unpacking using AVX, AVX2 and
BMI2 instructions through compiler intrinsics.

Vectorised bit unpacking is implemented for bit widths from 1 to 16.
Higher bit widths would not benefit from this vectorisation algorithm.

We check at runtime whether the required instructions are available on
the CPU and fall back to the scalar implementation if not.

The vectorised unpacking functions are in the file
be/src/util/bit-packing-vectorized.h, which is generated by the python
script in be/src/util/vectorised_bit_unpacking_generator.py.

Also adding benchmarks comparing the scalar and the vectorised
implementations.

Testing:
  - Added tests for the vectorised unpacking implementations.

Change-Id: I9e452a547973778bbd8d768c608e1a32e948f947
---
M be/src/benchmarks/bit-packing-benchmark.cc
M be/src/exec/parquet/parquet-bool-decoder.h
M be/src/util/bit-packing-test.cc
A be/src/util/bit-packing-vectorized.generated.h
M be/src/util/bit-packing.cc
M be/src/util/bit-packing.h
M be/src/util/bit-packing.inline.h
M be/src/util/bit-stream-utils-test.cc
M be/src/util/bit-stream-utils.h
M be/src/util/cpu-info.cc
M be/src/util/cpu-info.h
A be/src/util/vectorised_bit_unpacking_generator.py
12 files changed, 6,104 insertions(+), 98 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/07/13807/17
--
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: newpatchset
Gerrit-Change-Id: I9e452a547973778bbd8d768c608e1a32e948f947
Gerrit-Change-Number: 13807
Gerrit-PatchSet: 17
Gerrit-Owner: Daniel Becker 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Daniel Becker 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 


[Impala-ASF-CR] IMPALA-8741: Speed up bit unpacking by vectorisation

2019-07-24 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/13807 )

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


Patch Set 4:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/13807/4/be/src/util/bit-packing.inline.h
File be/src/util/bit-packing.inline.h:

http://gerrit.cloudera.org:8080/#/c/13807/4/be/src/util/bit-packing.inline.h@84
PS4, Line 84:   if (LIKELY((std::is_same::value
> I modified ParquetBoolDecoder and some tests to only allow unsigned integer
Thank you for doing this, nice to have fewer cases to handle in this 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: 4
Gerrit-Owner: Daniel Becker 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Daniel Becker 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Thu, 25 Jul 2019 00:17:46 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-8741: Speed up bit unpacking by vectorisation

2019-07-24 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/13807 )

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


Patch Set 16:

Build Failed

https://jenkins.impala.io/job/gerrit-code-review-checks/3980/ : Initial code 
review checks failed. See linked job for details on the failure.


--
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: 16
Gerrit-Owner: Daniel Becker 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Daniel Becker 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Wed, 24 Jul 2019 17:56:35 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-8741: Speed up bit unpacking by vectorisation

2019-07-24 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/13807 )

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


Patch Set 16:

(1 comment)

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

http://gerrit.cloudera.org:8080/#/c/13807/16/be/src/util/vectorised_bit_unpacking_generator.py@192
PS16, Line 192: =
flake8: E999 SyntaxError: invalid syntax



--
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: 16
Gerrit-Owner: Daniel Becker 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Daniel Becker 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Wed, 24 Jul 2019 17:36:58 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-8741: Speed up bit unpacking by vectorisation

2019-07-24 Thread Daniel Becker (Code Review)
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 16:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/13807/4/be/src/util/bit-packing.inline.h
File be/src/util/bit-packing.inline.h:

http://gerrit.cloudera.org:8080/#/c/13807/4/be/src/util/bit-packing.inline.h@84
PS4, Line 84: std::pair BitPacking::UnpackValues(
> Probably ParquetBoolDecoder::DecodeValue(). Might be worth seeing if you ca
I modified ParquetBoolDecoder and some tests to only allow unsigned integer 
types, it seems to work.



--
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: 16
Gerrit-Owner: Daniel Becker 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Daniel Becker 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Wed, 24 Jul 2019 17:36:22 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-8741: Speed up bit unpacking by vectorisation

2019-07-24 Thread Daniel Becker (Code Review)
Daniel Becker has uploaded a new patch set (#16). ( 
http://gerrit.cloudera.org:8080/13807 )

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

IMPALA-8741: Speed up bit unpacking by vectorisation

Adding a vectorised implementation to bit unpacking using AVX, AVX2 and
BMI2 instructions through compiler intrinsics.

Vectorised bit unpacking is implemented for bit widths from 1 to 16.
Higher bit widths would not benefit from this vectorisation algorithm.

We check at runtime whether the required instructions are available on
the CPU and fall back to the scalar implementation if not.

The vectorised unpacking functions are in the file
be/src/util/bit-packing-vectorized.h, which is generated by the python
script in be/src/util/vectorised_bit_unpacking_generator.py.

Also adding benchmarks comparing the scalar and the vectorised
implementations.

Testing:
  - Added tests for the vectorised unpacking implementations.

Change-Id: I9e452a547973778bbd8d768c608e1a32e948f947
---
M be/src/benchmarks/bit-packing-benchmark.cc
M be/src/exec/parquet/parquet-bool-decoder.h
M be/src/util/bit-packing-test.cc
A be/src/util/bit-packing-vectorized.h
M be/src/util/bit-packing.cc
M be/src/util/bit-packing.h
M be/src/util/bit-packing.inline.h
M be/src/util/bit-stream-utils-test.cc
M be/src/util/bit-stream-utils.h
M be/src/util/cpu-info.cc
M be/src/util/cpu-info.h
A be/src/util/vectorised_bit_unpacking_generator.py
12 files changed, 6,023 insertions(+), 98 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/07/13807/16
--
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: newpatchset
Gerrit-Change-Id: I9e452a547973778bbd8d768c608e1a32e948f947
Gerrit-Change-Number: 13807
Gerrit-PatchSet: 16
Gerrit-Owner: Daniel Becker 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Daniel Becker 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 


[Impala-ASF-CR] IMPALA-8741: Speed up bit unpacking by vectorisation

2019-07-24 Thread Csaba Ringhofer (Code Review)
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 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Daniel Becker 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Wed, 24 Jul 2019 17:32:05 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-8741: Speed up bit unpacking by vectorisation

2019-07-24 Thread Csaba Ringhofer (Code Review)
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 4:

> (2 comments)
 >
 > Thanks for the fixes. I think I'm going to struggle to find time to
 > review the core unpacking logic. Csaba, are you planning to review
 > that?

Sure, I have my hands on the unpacking logic, but the code generator is quite 
complex, so I didn't finish getting through it yet.


--
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: 4
Gerrit-Owner: Daniel Becker 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Daniel Becker 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Wed, 24 Jul 2019 14:00:10 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-8741: Speed up bit unpacking by vectorisation

2019-07-24 Thread Csaba Ringhofer (Code Review)
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 4:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/13807/15/be/src/benchmarks/bit-packing-benchmark.cc
File be/src/benchmarks/bit-packing-benchmark.cc:

http://gerrit.cloudera.org:8080/#/c/13807/15/be/src/benchmarks/bit-packing-benchmark.cc@744
PS15, Line 744:
  :
Is this intentional?


http://gerrit.cloudera.org:8080/#/c/13807/4/be/src/util/bit-packing-vectorized.h
File be/src/util/bit-packing-vectorized.h:

PS4:
> That is ok by me
Yes, I prefer committing the generated file in this case. I can imagine a 
situation where having the generated file in github can save someone a few 
minutes of working, e.g. in case of stacktrace pointing there. I am not afraid 
of bit rot in this case, as the generator is very self contained.

Unrelated: I would prefer to add "generated" or something similar to the file's 
name.


http://gerrit.cloudera.org:8080/#/c/13807/15/be/src/util/bit-packing.h
File be/src/util/bit-packing.h:

http://gerrit.cloudera.org:8080/#/c/13807/15/be/src/util/bit-packing.h@63
PS15, Line 63: ue,
nit: "the" not needed



--
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: 4
Gerrit-Owner: Daniel Becker 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Daniel Becker 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Wed, 24 Jul 2019 13:59:27 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-8741: Speed up bit unpacking by vectorisation

2019-07-23 Thread Tim Armstrong (Code Review)
Tim Armstrong 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)

Thanks for the fixes. I think I'm going to struggle to find time to review the 
core unpacking logic. Csaba, are you planning to review that?

http://gerrit.cloudera.org:8080/#/c/13807/4/be/src/util/bit-packing.h
File be/src/util/bit-packing.h:

http://gerrit.cloudera.org:8080/#/c/13807/4/be/src/util/bit-packing.h@67
PS4, Line 67:   template 
> Csaba had an idea that we could get rid of the VECTORIZE parameter complete
Ah if there is a measurable difference then the template parameter is OK by me, 
I just thought it might be fair enough outside the hot loops that it didn't 
make a difference. Either way it's a good improvement!


http://gerrit.cloudera.org:8080/#/c/13807/4/be/src/util/bit-packing.inline.h
File be/src/util/bit-packing.inline.h:

http://gerrit.cloudera.org:8080/#/c/13807/4/be/src/util/bit-packing.inline.h@84
PS4, Line 84:   if (LIKELY((std::is_same::value
> We also use bool somewhere, at least bit-packing.cc instantiates the method
Probably ParquetBoolDecoder::DecodeValue(). Might be worth seeing if you can 
switch that to storing an array of uint8_t and remove this.

I think supporting signed values in these low-level routines is probably adding 
unnecessary complexity.



--
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 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Daniel Becker 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Wed, 24 Jul 2019 00:53:10 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-8741: Speed up bit unpacking by vectorisation

2019-07-23 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/13807 )

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


Patch Set 4:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/13807/4/be/src/util/bit-packing-vectorized.h
File be/src/util/bit-packing-vectorized.h:

PS4:
> We could also take option 2 now and add a TODO to include the code generati
That is ok by me



--
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: 4
Gerrit-Owner: Daniel Becker 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Daniel Becker 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Wed, 24 Jul 2019 00:38:58 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-8741: Speed up bit unpacking by vectorisation

2019-07-23 Thread Daniel Becker (Code Review)
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 15:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/13807/4/be/src/util/bit-packing-vectorized.h
File be/src/util/bit-packing-vectorized.h:

PS4:
> Csaba prefers the second option.
We could also take option 2 now and add a TODO to include the code generation 
in the build process when we have Python 3 in the toolchain.



--
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 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Daniel Becker 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Tue, 23 Jul 2019 10:16:24 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-8741: Speed up bit unpacking by vectorisation

2019-07-23 Thread Daniel Becker (Code Review)
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 15:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/13807/4/be/src/util/bit-packing-vectorized.h
File be/src/util/bit-packing-vectorized.h:

PS4:
> I'll discuss it with other team members also.
Csaba prefers the second option.

I've partly ported the script to Python 2.6, but it requires some effort as 
some features I used in the script are not available in Python 2.6. Some of 
them are available as installable packages, but I guess installing them would 
complicate the build process too much.

Another argument in favour of the second approach is that if the generated code 
is checked in, it is possible to read it on Github and understand the code 
without having to build 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: 15
Gerrit-Owner: Daniel Becker 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Daniel Becker 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Tue, 23 Jul 2019 10:07:45 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-8741: Speed up bit unpacking by vectorisation

2019-07-23 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins 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:

Build Failed

https://jenkins.impala.io/job/gerrit-code-review-checks/3960/ : Initial code 
review checks failed. See linked job for details on the failure.


--
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 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Daniel Becker 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Tue, 23 Jul 2019 08:25:13 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-8741: Speed up bit unpacking by vectorisation

2019-07-23 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/13807 )

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


Patch Set 14:

Build Failed

https://jenkins.impala.io/job/gerrit-code-review-checks/3959/ : Initial code 
review checks failed. See linked job for details on the failure.


--
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: 14
Gerrit-Owner: Daniel Becker 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Daniel Becker 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Tue, 23 Jul 2019 08:16:45 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-8741: Speed up bit unpacking by vectorisation

2019-07-23 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/13807 )

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


Patch Set 13:

Build Failed

https://jenkins.impala.io/job/gerrit-code-review-checks/3957/ : Initial code 
review checks failed. See linked job for details on the failure.


--
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: 13
Gerrit-Owner: Daniel Becker 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Daniel Becker 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Tue, 23 Jul 2019 07:50:37 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-8741: Speed up bit unpacking by vectorisation

2019-07-23 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins 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:

(1 comment)

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@184
PS15, Line 184: =
flake8: E999 SyntaxError: invalid syntax



--
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 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Daniel Becker 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Tue, 23 Jul 2019 07:42:45 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-8741: Speed up bit unpacking by vectorisation

2019-07-23 Thread Daniel Becker (Code Review)
Daniel Becker has uploaded a new patch set (#15). ( 
http://gerrit.cloudera.org:8080/13807 )

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

IMPALA-8741: Speed up bit unpacking by vectorisation

Adding a vectorised implementation to bit unpacking using AVX, AVX2 and
BMI2 instructions through compiler intrinsics.

Vectorised bit unpacking is implemented for bit widths from 1 to 16.
Higher bit widths would not benefit from this vectorisation algorithm.

We check at runtime whether the required instructions are available on
the CPU and fall back to the scalar implementation if not.

The vectorised unpacking functions are in the file
be/src/util/bit-packing-vectorized.h, which is generated by the python
script in be/src/util/vectorised_bit_unpacking_generator.py.

Also adding benchmarks comparing the scalar and the vectorised
implementations.

Testing:
  - Added tests for the vectorised unpacking implementations.

Change-Id: I9e452a547973778bbd8d768c608e1a32e948f947
---
M be/src/benchmarks/bit-packing-benchmark.cc
M be/src/util/bit-packing-test.cc
A be/src/util/bit-packing-vectorized.h
M be/src/util/bit-packing.cc
M be/src/util/bit-packing.h
M be/src/util/bit-packing.inline.h
M be/src/util/cpu-info.cc
M be/src/util/cpu-info.h
A be/src/util/vectorised_bit_unpacking_generator.py
9 files changed, 5,977 insertions(+), 79 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/07/13807/15
--
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: newpatchset
Gerrit-Change-Id: I9e452a547973778bbd8d768c608e1a32e948f947
Gerrit-Change-Number: 13807
Gerrit-PatchSet: 15
Gerrit-Owner: Daniel Becker 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Daniel Becker 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 


[Impala-ASF-CR] IMPALA-8741: Speed up bit unpacking by vectorisation

2019-07-23 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/13807 )

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


Patch Set 14:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/13807/14/be/src/util/cpu-info.h
File be/src/util/cpu-info.h:

http://gerrit.cloudera.org:8080/#/c/13807/14/be/src/util/cpu-info.h@160
PS14, Line 160:   ///   // On the next line, the block closes, 'disabler's 
destructor runs, and AVX and AVX2
line too long (92 > 90)


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

http://gerrit.cloudera.org:8080/#/c/13807/14/be/src/util/vectorised_bit_unpacking_generator.py@184
PS14, Line 184: =
flake8: E999 SyntaxError: invalid syntax



--
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: 14
Gerrit-Owner: Daniel Becker 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Daniel Becker 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Tue, 23 Jul 2019 07:34:30 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-8741: Speed up bit unpacking by vectorisation

2019-07-23 Thread Daniel Becker (Code Review)
Daniel Becker has uploaded a new patch set (#14). ( 
http://gerrit.cloudera.org:8080/13807 )

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

IMPALA-8741: Speed up bit unpacking by vectorisation

Adding a vectorised implementation to bit unpacking using AVX, AVX2 and
BMI2 instructions through compiler intrinsics.

Vectorised bit unpacking is implemented for bit widths from 1 to 16.
Higher bit widths would not benefit from this vectorisation algorithm.

We check at runtime whether the required instructions are available on
the CPU and fall back to the scalar implementation if not.

The vectorised unpacking functions are in the file
be/src/util/bit-packing-vectorized.h, which is generated by the python
script in be/src/util/vectorised_bit_unpacking_generator.py.

Also adding benchmarks comparing the scalar and the vectorised
implementations.

Testing:
  - Added tests for the vectorised unpacking implementations.

Change-Id: I9e452a547973778bbd8d768c608e1a32e948f947
---
M be/src/benchmarks/bit-packing-benchmark.cc
M be/src/util/bit-packing-test.cc
A be/src/util/bit-packing-vectorized.h
M be/src/util/bit-packing.cc
M be/src/util/bit-packing.h
M be/src/util/bit-packing.inline.h
M be/src/util/cpu-info.cc
M be/src/util/cpu-info.h
A be/src/util/vectorised_bit_unpacking_generator.py
9 files changed, 5,976 insertions(+), 78 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/07/13807/14
--
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: newpatchset
Gerrit-Change-Id: I9e452a547973778bbd8d768c608e1a32e948f947
Gerrit-Change-Number: 13807
Gerrit-PatchSet: 14
Gerrit-Owner: Daniel Becker 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Daniel Becker 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 


[Impala-ASF-CR] IMPALA-8741: Speed up bit unpacking by vectorisation

2019-07-23 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/13807 )

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


Patch Set 13:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/13807/13/be/src/util/cpu-info.h
File be/src/util/cpu-info.h:

http://gerrit.cloudera.org:8080/#/c/13807/13/be/src/util/cpu-info.h@160
PS13, Line 160:   ///   // On the next line, the block closes, 'disabler's 
destructor runs, and AVX and AVX2
line too long (92 > 90)


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

http://gerrit.cloudera.org:8080/#/c/13807/13/be/src/util/vectorised_bit_unpacking_generator.py@184
PS13, Line 184: =
flake8: E999 SyntaxError: invalid syntax



--
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: 13
Gerrit-Owner: Daniel Becker 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Daniel Becker 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Tue, 23 Jul 2019 07:09:11 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-8741: Speed up bit unpacking by vectorisation

2019-07-23 Thread Daniel Becker (Code Review)
Daniel Becker has uploaded a new patch set (#13). ( 
http://gerrit.cloudera.org:8080/13807 )

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

IMPALA-8741: Speed up bit unpacking by vectorisation

Adding a vectorised implementation to bit unpacking using AVX, AVX2 and
BMI2 instructions through compiler intrinsics.

Vectorised bit unpacking is implemented for bit widths from 1 to 16.
Higher bit widths would not benefit from this vectorisation algorithm.

We check at runtime whether the required instructions are available on
the CPU and fall back to the scalar implementation if not.

The vectorised unpacking functions are in the file
be/src/util/bit-packing-vectorized.h, which is generated by the python
script in be/src/util/vectorised_bit_unpacking_generator.py.

Also adding benchmarks comparing the scalar and the vectorised
implementations.

Testing:
  - Added tests for the vectorised unpacking implementations.

Change-Id: I9e452a547973778bbd8d768c608e1a32e948f947
---
M be/src/benchmarks/bit-packing-benchmark.cc
M be/src/util/bit-packing-test.cc
A be/src/util/bit-packing-vectorized.h
M be/src/util/bit-packing.cc
M be/src/util/bit-packing.h
M be/src/util/bit-packing.inline.h
M be/src/util/cpu-info.cc
M be/src/util/cpu-info.h
A be/src/util/vectorised_bit_unpacking_generator.py
9 files changed, 5,976 insertions(+), 78 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/07/13807/13
--
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: newpatchset
Gerrit-Change-Id: I9e452a547973778bbd8d768c608e1a32e948f947
Gerrit-Change-Number: 13807
Gerrit-PatchSet: 13
Gerrit-Owner: Daniel Becker 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Daniel Becker 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 


[Impala-ASF-CR] IMPALA-8741: Speed up bit unpacking by vectorisation

2019-07-22 Thread Daniel Becker (Code Review)
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 12:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/13807/4/be/src/util/bit-packing.h
File be/src/util/bit-packing.h:

http://gerrit.cloudera.org:8080/#/c/13807/4/be/src/util/bit-packing.h@67
PS4, Line 67:   template 
> I made measurements and a graph and added it to the Jira issue:
Csaba had an idea that we could get rid of the VECTORIZE parameter completely 
and test the non-vectorised code path by disabling the CPU flags in CpuInfo in 
the tests.


http://gerrit.cloudera.org:8080/#/c/13807/4/be/src/util/bit-packing.inline.h
File be/src/util/bit-packing.inline.h:

http://gerrit.cloudera.org:8080/#/c/13807/4/be/src/util/bit-packing.inline.h@84
PS4, Line 84:   if (LIKELY((std::is_same::value
> Does it even make sense to unpack values into a different type outside of t
We also use bool somewhere, at least bit-packing.cc instantiates the methods 
with bool.



--
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: 12
Gerrit-Owner: Daniel Becker 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Daniel Becker 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Mon, 22 Jul 2019 11:47:06 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-8741: Speed up bit unpacking by vectorisation

2019-07-22 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/13807 )

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


Patch Set 12:

Build Failed

https://jenkins.impala.io/job/gerrit-code-review-checks/3945/ : Initial code 
review checks failed. See linked job for details on the failure.


--
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: 12
Gerrit-Owner: Daniel Becker 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Daniel Becker 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Mon, 22 Jul 2019 11:36:55 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-8741: Speed up bit unpacking by vectorisation

2019-07-22 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/13807 )

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


Patch Set 12:

(1 comment)

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

http://gerrit.cloudera.org:8080/#/c/13807/12/be/src/util/vectorised_bit_unpacking_generator.py@184
PS12, Line 184: =
flake8: E999 SyntaxError: invalid syntax



--
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: 12
Gerrit-Owner: Daniel Becker 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Daniel Becker 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Mon, 22 Jul 2019 10:54:21 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-8741: Speed up bit unpacking by vectorisation

2019-07-22 Thread Daniel Becker (Code Review)
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 12:

Added consts to avoid clang tidy warnings.


--
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: 12
Gerrit-Owner: Daniel Becker 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Daniel Becker 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Mon, 22 Jul 2019 10:53:52 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-8741: Speed up bit unpacking by vectorisation

2019-07-22 Thread Daniel Becker (Code Review)
Daniel Becker has uploaded a new patch set (#12). ( 
http://gerrit.cloudera.org:8080/13807 )

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

IMPALA-8741: Speed up bit unpacking by vectorisation

Adding a vectorised implementation to bit unpacking using AVX, AVX2 and
BMI2 instructions through compiler intrinsics.

Vectorised bit unpacking is implemented for bit widths from 1 to 16.
Higher bit widths would not benefit from this vectorisation algorithm.

We check at runtime whether the required instructions are available on
the CPU and fall back to the scalar implementation if not.

The vectorised unpacking functions are in the file
be/src/util/bit-packing-vectorized.h, which is generated by the python
script in be/src/util/vectorised_bit_unpacking_generator.py.

Also adding benchmarks comparing the scalar and the vectorised
implementations.

Testing:
  - Added tests for the vectorised unpacking implementations.

Change-Id: I9e452a547973778bbd8d768c608e1a32e948f947
---
M be/src/benchmarks/bit-packing-benchmark.cc
M be/src/util/bit-packing-test.cc
A be/src/util/bit-packing-vectorized.h
M be/src/util/bit-packing.cc
M be/src/util/bit-packing.h
M be/src/util/bit-packing.inline.h
M be/src/util/cpu-info.cc
M be/src/util/cpu-info.h
A be/src/util/vectorised_bit_unpacking_generator.py
9 files changed, 5,962 insertions(+), 73 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/07/13807/12
--
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: newpatchset
Gerrit-Change-Id: I9e452a547973778bbd8d768c608e1a32e948f947
Gerrit-Change-Number: 13807
Gerrit-PatchSet: 12
Gerrit-Owner: Daniel Becker 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Daniel Becker 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 


[Impala-ASF-CR] IMPALA-8741: Speed up bit unpacking by vectorisation

2019-07-22 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/13807 )

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


Patch Set 11:

Build Failed

https://jenkins.impala.io/job/gerrit-code-review-checks/3943/ : Initial code 
review checks failed. See linked job for details on the failure.


--
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: 11
Gerrit-Owner: Daniel Becker 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Daniel Becker 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Mon, 22 Jul 2019 10:07:26 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-8741: Speed up bit unpacking by vectorisation

2019-07-22 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins 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:

Build Failed

https://jenkins.impala.io/job/gerrit-code-review-checks/3941/ : Initial code 
review checks failed. See linked job for details on the failure.


--
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 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Daniel Becker 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Mon, 22 Jul 2019 09:49:42 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-8741: Speed up bit unpacking by vectorisation

2019-07-22 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/13807 )

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


Patch Set 11:

(1 comment)

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

http://gerrit.cloudera.org:8080/#/c/13807/11/be/src/util/vectorised_bit_unpacking_generator.py@184
PS11, Line 184: =
flake8: E999 SyntaxError: invalid syntax



--
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: 11
Gerrit-Owner: Daniel Becker 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Daniel Becker 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Mon, 22 Jul 2019 09:48:10 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-8741: Speed up bit unpacking by vectorisation

2019-07-22 Thread Daniel Becker (Code Review)
Daniel Becker has uploaded a new patch set (#11). ( 
http://gerrit.cloudera.org:8080/13807 )

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

IMPALA-8741: Speed up bit unpacking by vectorisation

Adding a vectorised implementation to bit unpacking using AVX, AVX2 and
BMI2 instructions through compiler intrinsics.

Vectorised bit unpacking is implemented for bit widths from 1 to 16.
Higher bit widths would not benefit from this vectorisation algorithm.

We check at runtime whether the required instructions are available on
the CPU and fall back to the scalar implementation if not.

The vectorised unpacking functions are in the file
be/src/util/bit-packing-vectorized.h, which is generated by the python
script in be/src/util/vectorised_bit_unpacking_generator.py.

Also adding benchmarks comparing the scalar and the vectorised
implementations.

Testing:
  - Added tests for the vectorised unpacking implementations.

Change-Id: I9e452a547973778bbd8d768c608e1a32e948f947
---
M be/src/benchmarks/bit-packing-benchmark.cc
M be/src/util/bit-packing-test.cc
A be/src/util/bit-packing-vectorized.h
M be/src/util/bit-packing.cc
M be/src/util/bit-packing.h
M be/src/util/bit-packing.inline.h
M be/src/util/cpu-info.cc
M be/src/util/cpu-info.h
A be/src/util/vectorised_bit_unpacking_generator.py
9 files changed, 5,962 insertions(+), 73 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/07/13807/11
--
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: newpatchset
Gerrit-Change-Id: I9e452a547973778bbd8d768c608e1a32e948f947
Gerrit-Change-Number: 13807
Gerrit-PatchSet: 11
Gerrit-Owner: Daniel Becker 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Daniel Becker 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 


[Impala-ASF-CR] IMPALA-8741: Speed up bit unpacking by vectorisation

2019-07-22 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins 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:

(1 comment)

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

http://gerrit.cloudera.org:8080/#/c/13807/10/be/src/util/vectorised_bit_unpacking_generator.py@184
PS10, Line 184: =
flake8: E999 SyntaxError: invalid syntax



--
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 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Daniel Becker 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Mon, 22 Jul 2019 09:08:06 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-8741: Speed up bit unpacking by vectorisation

2019-07-22 Thread Daniel Becker (Code Review)
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 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Daniel Becker 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Mon, 22 Jul 2019 09:07:33 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-8741: Speed up bit unpacking by vectorisation

2019-07-22 Thread Daniel Becker (Code Review)
Daniel Becker has uploaded a new patch set (#10). ( 
http://gerrit.cloudera.org:8080/13807 )

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

IMPALA-8741: Speed up bit unpacking by vectorisation

Adding a vectorised implementation to bit unpacking using AVX, AVX2 and
BMI2 instructions through compiler intrinsics.

Vectorised bit unpacking is implemented for bit widths from 1 to 16.
Higher bit widths would not benefit from this vectorisation algorithm.

We check at runtime whether the required instructions are available on
the CPU and fall back to the scalar implementation if not.

The vectorised unpacking functions are in the file
be/src/util/bit-packing-vectorized.h, which is generated by the python
script in be/src/util/vectorised_bit_unpacking_generator.py.

Also adding benchmarks comparing the scalar and the vectorised
implementations.

Testing:
  - Added tests for the vectorised unpacking implementations.

Change-Id: I9e452a547973778bbd8d768c608e1a32e948f947
---
M be/src/benchmarks/bit-packing-benchmark.cc
M be/src/util/bit-packing-test.cc
A be/src/util/bit-packing-vectorized.h
M be/src/util/bit-packing.cc
M be/src/util/bit-packing.h
M be/src/util/bit-packing.inline.h
M be/src/util/cpu-info.cc
M be/src/util/cpu-info.h
A be/src/util/vectorised_bit_unpacking_generator.py
9 files changed, 5,962 insertions(+), 73 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/07/13807/10
--
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: newpatchset
Gerrit-Change-Id: I9e452a547973778bbd8d768c608e1a32e948f947
Gerrit-Change-Number: 13807
Gerrit-PatchSet: 10
Gerrit-Owner: Daniel Becker 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Daniel Becker 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 


[Impala-ASF-CR] IMPALA-8741: Speed up bit unpacking by vectorisation

2019-07-19 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/13807 )

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


Patch Set 4:

(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:
> We thought it would make the build process more complicated if we generated
Ahh I see. I don't think it should be too bad, we have similar CMake rules, 
e.g. see the file2array invocations in e/src/codegen/CMakeLists. Having a 
reproducible build process ultimately simplifies maintenance even if it 
requires some upfront work.

That said, I don't think we expect this file to change that often so requiring 
a manual rerun when the python script changes isn't unreasonable.

Taking into account this and the Python 3 issue, I think we have two paths 
forward:
* Port the script to Python 2 (we don't have Python 3 on all systems we build 
on yet; would probably have to add it to the toolchain, which I would like to 
do eventually to get us off Python 2 for tests, etc) and add a CMake rule to 
generate in each build
* Keep this as is, but ensure that the code generation is easily reproducible 
by others. Maybe it would be enough to generate a comment in this files header 
with the vectorised_bit_unpacking_generator.py command-line used.

I somewhat prefer the first just for reproducibility, but I'm open to the 
second. The main downside of the first is the extra work to port the script, 
since we're going to have to port it back to Python 3 at some point in the 
future. I think if others prefer the second we should go with that.


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 canonical 
behaviour. If the output is readable without clang-format, we could simplify 
the script. Otherwise we could just always run through clang-format.


http://gerrit.cloudera.org:8080/#/c/13807/8/be/src/util/vectorised_bit_unpacking_generator.py@1544
PS8, Line 1544: else:
If we're going with clang-format, I think we should write the non-formatted 
output to a temporary file, then run it through clang format. Overwriting files 
in place as part of a build step doesn't play nice with a lot of build systems 
like make, since if the command fails partway through, make will just look at 
the timestamps and think that the output was correctly generated.


http://gerrit.cloudera.org:8080/#/c/13807/8/be/src/util/vectorised_bit_unpacking_generator.py@1547
PS8, Line 1547: if __name__ == "__main__":
I think the script should just fail if clang_format was enabled and couldn't be 
found - the script didn't do what the user asked for, so probably shouldn't do 
anything.



--
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: 4
Gerrit-Owner: Daniel Becker 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Daniel Becker 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Fri, 19 Jul 2019 17:13:46 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-8741: Speed up bit unpacking by vectorisation

2019-07-19 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/13807 )

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


Patch Set 8:

Build Failed

https://jenkins.impala.io/job/gerrit-code-review-checks/3920/ : Initial code 
review checks failed. See linked job for details on the failure.


--
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: 8
Gerrit-Owner: Daniel Becker 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Daniel Becker 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Fri, 19 Jul 2019 14:51:40 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-8741: Speed up bit unpacking by vectorisation

2019-07-19 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/13807 )

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


Patch Set 8:

(1 comment)

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@183
PS8, Line 183: =
flake8: E999 SyntaxError: invalid syntax



--
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: 8
Gerrit-Owner: Daniel Becker 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Daniel Becker 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Fri, 19 Jul 2019 14:11:47 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-8741: Speed up bit unpacking by vectorisation

2019-07-19 Thread Daniel Becker (Code Review)
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 8:

Trying to avoid casting away constness.


--
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: 8
Gerrit-Owner: Daniel Becker 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Daniel Becker 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Fri, 19 Jul 2019 14:11:33 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-8741: Speed up bit unpacking by vectorisation

2019-07-19 Thread Daniel Becker (Code Review)
Daniel Becker has uploaded a new patch set (#8). ( 
http://gerrit.cloudera.org:8080/13807 )

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

IMPALA-8741: Speed up bit unpacking by vectorisation

Adding a vectorised implementation to bit unpacking using AVX, AVX2 and
BMI2 instructions through compiler intrinsics.

Vectorised bit unpacking is implemented for bit widths from 1 to 16.
Higher bit widths would not benefit from this vectorisation algorithm.

We check at runtime whether the required instructions are available on
the CPU and fall back to the scalar implementation if not.

The vectorised unpacking functions are in the file
be/src/util/bit-packing-vectorized.h, which is generated by the python
script in be/src/util/vectorised_bit_unpacking_generator.py.

Also adding benchmarks comparing the scalar and the vectorised
implementations.

Testing:
  - Added tests for the vectorised unpacking implementations.

Change-Id: I9e452a547973778bbd8d768c608e1a32e948f947
---
M be/src/benchmarks/bit-packing-benchmark.cc
M be/src/util/bit-packing-test.cc
A be/src/util/bit-packing-vectorized.h
M be/src/util/bit-packing.cc
M be/src/util/bit-packing.h
M be/src/util/bit-packing.inline.h
M be/src/util/cpu-info.cc
M be/src/util/cpu-info.h
A be/src/util/vectorised_bit_unpacking_generator.py
9 files changed, 5,957 insertions(+), 73 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/07/13807/8
--
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: newpatchset
Gerrit-Change-Id: I9e452a547973778bbd8d768c608e1a32e948f947
Gerrit-Change-Number: 13807
Gerrit-PatchSet: 8
Gerrit-Owner: Daniel Becker 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Daniel Becker 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 


[Impala-ASF-CR] IMPALA-8741: Speed up bit unpacking by vectorisation

2019-07-19 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/13807 )

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


Patch Set 7:

Build Failed

https://jenkins.impala.io/job/gerrit-code-review-checks/3919/ : Initial code 
review checks failed. See linked job for details on the failure.


--
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: 7
Gerrit-Owner: Daniel Becker 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Daniel Becker 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Fri, 19 Jul 2019 13:50:10 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-8741: Speed up bit unpacking by vectorisation

2019-07-19 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/13807 )

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


Patch Set 6:

Build Failed

https://jenkins.impala.io/job/gerrit-code-review-checks/3918/ : Initial code 
review checks failed. See linked job for details on the failure.


--
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: 6
Gerrit-Owner: Daniel Becker 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Daniel Becker 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Fri, 19 Jul 2019 13:19:31 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-8741: Speed up bit unpacking by vectorisation

2019-07-19 Thread Daniel Becker (Code Review)
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 7:

(1 comment)

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

http://gerrit.cloudera.org:8080/#/c/13807/7/be/src/util/vectorised_bit_unpacking_generator.py@183
PS7, Line 183: =
> flake8: E999 SyntaxError: invalid syntax
This python script uses python3. Should I translate it to python2?



--
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: 7
Gerrit-Owner: Daniel Becker 
Gerrit-Reviewer: Daniel Becker 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Fri, 19 Jul 2019 13:12:05 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-8741: Speed up bit unpacking by vectorisation

2019-07-19 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/13807 )

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


Patch Set 7:

(1 comment)

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

http://gerrit.cloudera.org:8080/#/c/13807/7/be/src/util/vectorised_bit_unpacking_generator.py@183
PS7, Line 183: =
flake8: E999 SyntaxError: invalid syntax



--
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: 7
Gerrit-Owner: Daniel Becker 
Gerrit-Reviewer: Daniel Becker 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Fri, 19 Jul 2019 13:09:42 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-8741: Speed up bit unpacking by vectorisation

2019-07-19 Thread Daniel Becker (Code Review)
Daniel Becker has uploaded a new patch set (#7). ( 
http://gerrit.cloudera.org:8080/13807 )

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

IMPALA-8741: Speed up bit unpacking by vectorisation

Adding a vectorised implementation to bit unpacking using AVX, AVX2 and
BMI2 instructions through compiler intrinsics.

Vectorised bit unpacking is implemented for bit widths from 1 to 16.
Higher bit widths would not benefit from this vectorisation algorithm.

We check at runtime whether the required instructions are available on
the CPU and fall back to the scalar implementation if not.

The vectorised unpacking functions are in the file
be/src/util/bit-packing-vectorized.h, which is generated by the python
script in be/src/util/vectorised_bit_unpacking_generator.py.

Also adding benchmarks comparing the scalar and the vectorised
implementations.

Testing:
  - Added tests for the vectorised unpacking implementations.

Change-Id: I9e452a547973778bbd8d768c608e1a32e948f947
---
M be/src/benchmarks/bit-packing-benchmark.cc
M be/src/util/bit-packing-test.cc
A be/src/util/bit-packing-vectorized.h
M be/src/util/bit-packing.cc
M be/src/util/bit-packing.h
M be/src/util/bit-packing.inline.h
M be/src/util/cpu-info.cc
M be/src/util/cpu-info.h
A be/src/util/vectorised_bit_unpacking_generator.py
9 files changed, 5,955 insertions(+), 73 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/07/13807/7
--
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: newpatchset
Gerrit-Change-Id: I9e452a547973778bbd8d768c608e1a32e948f947
Gerrit-Change-Number: 13807
Gerrit-PatchSet: 7
Gerrit-Owner: Daniel Becker 
Gerrit-Reviewer: Daniel Becker 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 


[Impala-ASF-CR] IMPALA-8741: Speed up bit unpacking by vectorisation

2019-07-19 Thread Daniel Becker (Code Review)
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 7:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/13807/4/be/src/benchmarks/bit-packing-benchmark.cc
File be/src/benchmarks/bit-packing-benchmark.cc:

http://gerrit.cloudera.org:8080/#/c/13807/4/be/src/benchmarks/bit-packing-benchmark.cc@29
PS4, Line 29: // The second one compares the original scalar implementation 
with the vectorised one
> Include results for this one as a comment?
Done


http://gerrit.cloudera.org:8080/#/c/13807/4/be/src/util/bit-packing.inline.h
File be/src/util/bit-packing.inline.h:

http://gerrit.cloudera.org:8080/#/c/13807/4/be/src/util/bit-packing.inline.h@262
PS4, Line 262:
> indentation
Done



--
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: 7
Gerrit-Owner: Daniel Becker 
Gerrit-Reviewer: Daniel Becker 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Fri, 19 Jul 2019 13:09:11 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-8741: Speed up bit unpacking by vectorisation

2019-07-19 Thread Daniel Becker (Code Review)
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 6:

(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:
> We generally don't check in generated files. There are arguments for and ag
We thought it would make the build process more complicated if we generated the 
file during the build. On the other hand, we already have generated files so 
maybe this is not a problem.


http://gerrit.cloudera.org:8080/#/c/13807/4/be/src/util/bit-packing.h
File be/src/util/bit-packing.h:

http://gerrit.cloudera.org:8080/#/c/13807/4/be/src/util/bit-packing.h@67
PS4, Line 67:   template 
> Is there a significant performance benefit to making VECTORIZE a compile-ti
I made measurements and a graph and added it to the Jira issue:

IMPALA-8741

With a different measurement, the gap was a little bigger. Do you think it's 
not worth the template parameter?


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

http://gerrit.cloudera.org:8080/#/c/13807/4/be/src/util/vectorised_bit_unpacking_generator.py@142
PS4, Line 142: single
> single
Done


http://gerrit.cloudera.org:8080/#/c/13807/4/be/src/util/vectorised_bit_unpacking_generator.py@1080
PS4, Line 1080: metho
> method
Done



--
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: 6
Gerrit-Owner: Daniel Becker 
Gerrit-Reviewer: Daniel Becker 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Fri, 19 Jul 2019 12:46:48 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-8741: Speed up bit unpacking by vectorisation

2019-07-19 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/13807 )

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


Patch Set 6:

(1 comment)

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

http://gerrit.cloudera.org:8080/#/c/13807/6/be/src/util/vectorised_bit_unpacking_generator.py@183
PS6, Line 183: =
flake8: E999 SyntaxError: invalid syntax



--
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: 6
Gerrit-Owner: Daniel Becker 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Fri, 19 Jul 2019 12:39:18 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-8741: Speed up bit unpacking by vectorisation

2019-07-19 Thread Daniel Becker (Code Review)
Daniel Becker has uploaded a new patch set (#6). ( 
http://gerrit.cloudera.org:8080/13807 )

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

IMPALA-8741: Speed up bit unpacking by vectorisation

Adding a vectorised implementation to bit unpacking using AVX, AVX2 and
BMI2 instructions through compiler intrinsics.

Vectorised bit unpacking is implemented for bit widths from 1 to 16.
Higher bit widths would not benefit from this vectorisation algorithm.

We check at runtime whether the required instructions are available on
the CPU and fall back to the scalar implementation if not.

The vectorised unpacking functions are in the file
be/src/util/bit-packing-vectorized.h, which is generated by the python
script in be/src/util/vectorised_bit_unpacking_generator.py.

Also adding benchmarks comparing the scalar and the vectorised
implementations.

Testing:
  - Added tests for the vectorised unpacking implementations.

Change-Id: I9e452a547973778bbd8d768c608e1a32e948f947
---
M be/src/benchmarks/bit-packing-benchmark.cc
M be/src/util/bit-packing-test.cc
A be/src/util/bit-packing-vectorized.h
M be/src/util/bit-packing.cc
M be/src/util/bit-packing.h
M be/src/util/bit-packing.inline.h
M be/src/util/cpu-info.cc
M be/src/util/cpu-info.h
A be/src/util/vectorised_bit_unpacking_generator.py
9 files changed, 5,775 insertions(+), 73 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/07/13807/6
--
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: newpatchset
Gerrit-Change-Id: I9e452a547973778bbd8d768c608e1a32e948f947
Gerrit-Change-Number: 13807
Gerrit-PatchSet: 6
Gerrit-Owner: Daniel Becker 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 


[Impala-ASF-CR] IMPALA-8741: Speed up bit unpacking by vectorisation

2019-07-17 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/13807 )

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


Patch Set 4:

(8 comments)

I started reviewing this morning but ran out of time to look today. I got 
through the C++ code but haven't reviewed the guts of the code generator. I 
have pretty good confidence it's correct though, the unit tests should provide 
good coverage.

http://gerrit.cloudera.org:8080/#/c/13807/4/be/src/benchmarks/bit-packing-benchmark.cc
File be/src/benchmarks/bit-packing-benchmark.cc:

http://gerrit.cloudera.org:8080/#/c/13807/4/be/src/benchmarks/bit-packing-benchmark.cc@29
PS4, Line 29: // The second one compares the original scalar implementation 
with the vectorised one
Include results for this one as a comment?


http://gerrit.cloudera.org:8080/#/c/13807/4/be/src/util/bit-packing-vectorized.h
File be/src/util/bit-packing-vectorized.h:

PS4:
We generally don't check in generated files. There are arguments for and 
against doing so, but generally that's the direction we've gone. The most 
compelling reason for me is that re-generating the code as part of the build 
means that  vectorised_bit_unpacking_generator.py is tested. Otherwise it could 
easily bit rot.

I think this is useful for the purposes of review, but I'd be inclined to 
remove it before merging and rely on generating via a CMake rule. We can 
discuss the pros and cons though; maybe there are some considerations I'm 
issing.


http://gerrit.cloudera.org:8080/#/c/13807/4/be/src/util/bit-packing.h
File be/src/util/bit-packing.h:

http://gerrit.cloudera.org:8080/#/c/13807/4/be/src/util/bit-packing.h@64
PS4, Line 64: simultaniously
simultaneously


http://gerrit.cloudera.org:8080/#/c/13807/4/be/src/util/bit-packing.h@67
PS4, Line 67:   template 
Is there a significant performance benefit to making VECTORIZE a compile-time 
constant - we already have to do a runtime check for the instruction anyway, so 
it can't result in more specialisation.


http://gerrit.cloudera.org:8080/#/c/13807/4/be/src/util/bit-packing.inline.h
File be/src/util/bit-packing.inline.h:

http://gerrit.cloudera.org:8080/#/c/13807/4/be/src/util/bit-packing.inline.h@84
PS4, Line 84:   if (LIKELY((std::is_same::value
Does it even make sense to unpack values into a different type outside of these 
4? Could we make this a static_assert instead?

That would avoid someone accidentally instantiating a non-vectorized version.


http://gerrit.cloudera.org:8080/#/c/13807/4/be/src/util/bit-packing.inline.h@262
PS4, Line 262: return in;
indentation


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

http://gerrit.cloudera.org:8080/#/c/13807/4/be/src/util/vectorised_bit_unpacking_generator.py@142
PS4, Line 142: sinlge
single


http://gerrit.cloudera.org:8080/#/c/13807/4/be/src/util/vectorised_bit_unpacking_generator.py@1080
PS4, Line 1080: metod
method



--
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: 4
Gerrit-Owner: Daniel Becker 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Wed, 17 Jul 2019 23:02:01 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-8741: Speed up bit unpacking by vectorisation

2019-07-10 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/13807 )

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


Patch Set 4:

Build Failed

https://jenkins.impala.io/job/gerrit-code-review-checks/3857/ : Initial code 
review checks failed. See linked job for details on the failure.


--
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: 4
Gerrit-Owner: Daniel Becker 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Comment-Date: Wed, 10 Jul 2019 11:46:06 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-8741: Speed up bit unpacking by vectorisation

2019-07-10 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/13807 )

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


Patch Set 4:

(2 comments)

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

http://gerrit.cloudera.org:8080/#/c/13807/4/be/src/util/vectorised_bit_unpacking_generator.py@183
PS4, Line 183: =
flake8: E999 SyntaxError: invalid syntax


http://gerrit.cloudera.org:8080/#/c/13807/4/be/src/util/vectorised_bit_unpacking_generator.py@1547
PS4, Line 1547: if __name__ == "__main__":
flake8: E305 expected 2 blank lines after class or function definition, found 1



--
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: 4
Gerrit-Owner: Daniel Becker 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Comment-Date: Wed, 10 Jul 2019 11:06:16 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-8741: Speed up bit unpacking by vectorisation

2019-07-10 Thread Daniel Becker (Code Review)
Daniel Becker has uploaded a new patch set (#4). ( 
http://gerrit.cloudera.org:8080/13807 )

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

IMPALA-8741: Speed up bit unpacking by vectorisation

Adding a vectorised implementation to bit unpacking using AVX, AVX2 and
BMI2 instructions through compiler intrinsics.

Vectorised bit unpacking is implemented for bit widths from 1 to 16.
Higher bit widths would not benefit from this vectorisation algorithm.

We check at runtime whether the required instructions are available on
the CPU and fall back to the scalar implementation if not.

The vectorised unpacking functions are in the file
be/src/util/bit-packing-vectorized.h, which is generated by the python
script in be/src/util/vectorised_bit_unpacking_generator.py.

Also adding benchmarks comparing the scalar and the vectorised
implementations.

Testing:
  - Added tests for the vectorised unpacking implementations.

Change-Id: I9e452a547973778bbd8d768c608e1a32e948f947
---
M be/src/benchmarks/bit-packing-benchmark.cc
M be/src/util/bit-packing-test.cc
A be/src/util/bit-packing-vectorized.h
M be/src/util/bit-packing.cc
M be/src/util/bit-packing.h
M be/src/util/bit-packing.inline.h
M be/src/util/cpu-info.cc
M be/src/util/cpu-info.h
A be/src/util/vectorised_bit_unpacking_generator.py
9 files changed, 5,771 insertions(+), 73 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/07/13807/4
--
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: newpatchset
Gerrit-Change-Id: I9e452a547973778bbd8d768c608e1a32e948f947
Gerrit-Change-Number: 13807
Gerrit-PatchSet: 4
Gerrit-Owner: Daniel Becker 
Gerrit-Reviewer: Impala Public Jenkins 


[Impala-ASF-CR] IMPALA-8741: Speed up bit unpacking by vectorisation

2019-07-05 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/13807 )

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


Patch Set 3:

Build Failed

https://jenkins.impala.io/job/gerrit-code-review-checks/3829/ : Initial code 
review checks failed. See linked job for details on the failure.


--
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: 3
Gerrit-Owner: Daniel Becker 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Comment-Date: Fri, 05 Jul 2019 14:49:54 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-8741: Speed up bit unpacking by vectorisation

2019-07-05 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/13807 )

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


Patch Set 3:

(186 comments)

http://gerrit.cloudera.org:8080/#/c/13807/3/be/src/util/bit-packing-test.cc
File be/src/util/bit-packing-test.cc:

http://gerrit.cloudera.org:8080/#/c/13807/3/be/src/util/bit-packing-test.cc@110
PS3, Line 110:   EXPECT_EQ(in[i] & mask, out[i]) << "Didn't get back input 
value " << i << ". Bit width: " << bit_width << ".";
line too long (116 > 90)


http://gerrit.cloudera.org:8080/#/c/13807/3/be/src/util/bit-packing.inline.h
File be/src/util/bit-packing.inline.h:

http://gerrit.cloudera.org:8080/#/c/13807/3/be/src/util/bit-packing.inline.h@91
PS3, Line 91: in_pos = UnpackInBatchesOf32Vectorized(batches_to_read, in_pos, out_pos);
line too long (97 > 90)


http://gerrit.cloudera.org:8080/#/c/13807/3/be/src/util/bit-packing.inline.h@93
PS3, Line 93: in_pos = UnpackInBatchesOf32(batches_to_read, in_pos, in_bytes, out_pos);
line too long (97 > 90)


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

http://gerrit.cloudera.org:8080/#/c/13807/3/be/src/util/vectorised_bit_unpacking_generator.py@25
PS3, Line 25:
flake8: E261 at least two spaces before inline comment


http://gerrit.cloudera.org:8080/#/c/13807/3/be/src/util/vectorised_bit_unpacking_generator.py@33
PS3, Line 33:
flake8: E261 at least two spaces before inline comment


http://gerrit.cloudera.org:8080/#/c/13807/3/be/src/util/vectorised_bit_unpacking_generator.py@35
PS3, Line 35: @unique
flake8: E302 expected 2 blank lines, found 1


http://gerrit.cloudera.org:8080/#/c/13807/3/be/src/util/vectorised_bit_unpacking_generator.py@44
PS3, Line 44: @unique
flake8: E302 expected 2 blank lines, found 1


http://gerrit.cloudera.org:8080/#/c/13807/3/be/src/util/vectorised_bit_unpacking_generator.py@74
PS3, Line 74: class UnpackParams:
flake8: E302 expected 2 blank lines, found 1


http://gerrit.cloudera.org:8080/#/c/13807/3/be/src/util/vectorised_bit_unpacking_generator.py@90
PS3, Line 90: e
flake8: E501 line too long (98 > 90 characters)


http://gerrit.cloudera.org:8080/#/c/13807/3/be/src/util/vectorised_bit_unpacking_generator.py@94
PS3, Line 94: o
flake8: E501 line too long (100 > 90 characters)


http://gerrit.cloudera.org:8080/#/c/13807/3/be/src/util/vectorised_bit_unpacking_generator.py@129
PS3, Line 129: class GeneratedCode:
flake8: E302 expected 2 blank lines, found 1


http://gerrit.cloudera.org:8080/#/c/13807/3/be/src/util/vectorised_bit_unpacking_generator.py@138
PS3, Line 138: a
flake8: E501 line too long (92 > 90 characters)


http://gerrit.cloudera.org:8080/#/c/13807/3/be/src/util/vectorised_bit_unpacking_generator.py@139
PS3, Line 139: c
flake8: E501 line too long (99 > 90 characters)


http://gerrit.cloudera.org:8080/#/c/13807/3/be/src/util/vectorised_bit_unpacking_generator.py@140
PS3, Line 140: u
flake8: E501 line too long (97 > 90 characters)


http://gerrit.cloudera.org:8080/#/c/13807/3/be/src/util/vectorised_bit_unpacking_generator.py@150
PS3, Line 150: )
flake8: E501 line too long (91 > 90 characters)


http://gerrit.cloudera.org:8080/#/c/13807/3/be/src/util/vectorised_bit_unpacking_generator.py@163
PS3, Line 163: h
flake8: E501 line too long (99 > 90 characters)


http://gerrit.cloudera.org:8080/#/c/13807/3/be/src/util/vectorised_bit_unpacking_generator.py@172
PS3, Line 172: e
flake8: E501 line too long (98 > 90 characters)


http://gerrit.cloudera.org:8080/#/c/13807/3/be/src/util/vectorised_bit_unpacking_generator.py@177
PS3, Line 177: class UnpackMethod(metaclass=ABCMeta):
flake8: E302 expected 2 blank lines, found 1


http://gerrit.cloudera.org:8080/#/c/13807/3/be/src/util/vectorised_bit_unpacking_generator.py@177
PS3, Line 177: =
flake8: E999 SyntaxError: invalid syntax


http://gerrit.cloudera.org:8080/#/c/13807/3/be/src/util/vectorised_bit_unpacking_generator.py@186
PS3, Line 186: o
flake8: E501 line too long (96 > 90 characters)


http://gerrit.cloudera.org:8080/#/c/13807/3/be/src/util/vectorised_bit_unpacking_generator.py@194
PS3, Line 194: e
flake8: E501 line too long (96 > 90 characters)


http://gerrit.cloudera.org:8080/#/c/13807/3/be/src/util/vectorised_bit_unpacking_generator.py@201
PS3, Line 201: t
flake8: E501 line too long (98 > 90 characters)


http://gerrit.cloudera.org:8080/#/c/13807/3/be/src/util/vectorised_bit_unpacking_generator.py@202
PS3, Line 202: t
flake8: E501 line too long (99 > 90 characters)


http://gerrit.cloudera.org:8080/#/c/13807/3/be/src/util/vectorised_bit_unpacking_generator.py@206
PS3, Line 206: class UnpackStage:
flake8: E302 expected 2 blank lines, found 1


http://gerrit.cloudera.org:8080/#/c/13807/3/be/src/util/vectorised_bit_unpacking_generator.py@208
PS3, Line 208: o
flake8: E501 line too long (100 > 90 characters)



[Impala-ASF-CR] IMPALA-8741: Speed up bit unpacking by vectorisation

2019-07-05 Thread Daniel Becker (Code Review)
Daniel Becker has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/13807


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

IMPALA-8741: Speed up bit unpacking by vectorisation

Adding a vectorised implementation to bit unpacking using AVX, AVX2 and
BMI2 instructions through compiler intrinsics.

Vectorised bit unpacking is implemented for bit widths from 1 to 16.
Higher bit widths would not benefit from this vectorisation algorithm.

We check at runtime whether the required instructions are available on
the CPU and fall back to the scalar implementation if not.

The vectorised unpacking functions are in the file
be/src/util/bit-packing-vectorized.h, which is generated by the python
script in be/src/util/vectorised_bit_unpacking_generator.py.

Also adding benchmarks comparing the scalar and the vectorised
implementations.

Testing:
  - Added tests for the vectorised unpacking implementations.

Change-Id: I9e452a547973778bbd8d768c608e1a32e948f947
---
M be/src/benchmarks/bit-packing-benchmark.cc
M be/src/util/bit-packing-test.cc
A be/src/util/bit-packing-vectorized.h
M be/src/util/bit-packing.cc
M be/src/util/bit-packing.h
M be/src/util/bit-packing.inline.h
M be/src/util/cpu-info.cc
M be/src/util/cpu-info.h
A be/src/util/vectorised_bit_unpacking_generator.py
9 files changed, 5,704 insertions(+), 73 deletions(-)



  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/07/13807/3
--
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: newchange
Gerrit-Change-Id: I9e452a547973778bbd8d768c608e1a32e948f947
Gerrit-Change-Number: 13807
Gerrit-PatchSet: 3
Gerrit-Owner: Daniel Becker