[Impala-ASF-CR] IMPALA-8710: Increase allowed bit width to 64 for bit packing
Csaba Ringhofer has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/13809 ) Change subject: IMPALA-8710: Increase allowed bit width to 64 for bit packing .. IMPALA-8710: Increase allowed bit width to 64 for bit packing Fixed the buffer overflow that the previous attempt (commit b1cbf9e6b786132e86699cbb1e472ec98499bb11, https://gerrit.cloudera.org/#/c/13737) introduced. Compared to that change, only bit-packing.inline.h is different. The tests went into the buffer overflow path but it only produced an error in the ASAN builds. Increasing the allowed bit width for bit packing and bit unpacking to 64 bits. This will be needed to support the Parquet delta encoding. Added new methods to BitWriter and BatchedBitReader handling Uleb and ZigZag integers for 64 bits, also needed by delta encoding. The performance of bit unpacking is either the same or better with the new implementation, depending on bit width. Updated the results in bit-packing-benchmark.cc. Testing: - Modified bit packing and unpacking tests to test bit widths up to 64 bits. - Tests covering the additions in BitWriter and BatchedBitReader. Change-Id: Ib9276ca291e3d36db16f63f8caf3e9248a18d85b Reviewed-on: http://gerrit.cloudera.org:8080/13809 Reviewed-by: Impala Public Jenkins Tested-by: Impala Public Jenkins --- M be/src/benchmarks/bit-packing-benchmark.cc M be/src/util/CMakeLists.txt M be/src/util/bit-packing-test.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/bit-stream-utils.inline.h M be/src/util/rle-encoding.h M be/src/util/rle-test.cc 10 files changed, 430 insertions(+), 209 deletions(-) Approvals: Impala Public Jenkins: Looks good to me, approved; Verified -- To view, visit http://gerrit.cloudera.org:8080/13809 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: Ib9276ca291e3d36db16f63f8caf3e9248a18d85b Gerrit-Change-Number: 13809 Gerrit-PatchSet: 14 Gerrit-Owner: Daniel Becker Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Zoltan Borok-Nagy
[Impala-ASF-CR] IMPALA-8710: Increase allowed bit width to 64 for bit packing
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13809 ) Change subject: IMPALA-8710: Increase allowed bit width to 64 for bit packing .. Patch Set 13: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/3973/ : 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/13809 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ib9276ca291e3d36db16f63f8caf3e9248a18d85b Gerrit-Change-Number: 13809 Gerrit-PatchSet: 13 Gerrit-Owner: Daniel Becker Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Wed, 24 Jul 2019 09:18:18 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8710: Increase allowed bit width to 64 for bit packing
Daniel Becker has uploaded a new patch set (#13). ( http://gerrit.cloudera.org:8080/13809 ) Change subject: IMPALA-8710: Increase allowed bit width to 64 for bit packing .. IMPALA-8710: Increase allowed bit width to 64 for bit packing Fixed the buffer overflow that the previous attempt (commit b1cbf9e6b786132e86699cbb1e472ec98499bb11, https://gerrit.cloudera.org/#/c/13737) introduced. Compared to that change, only bit-packing.inline.h is different. The tests went into the buffer overflow path but it only produced an error in the ASAN builds. Increasing the allowed bit width for bit packing and bit unpacking to 64 bits. This will be needed to support the Parquet delta encoding. Added new methods to BitWriter and BatchedBitReader handling Uleb and ZigZag integers for 64 bits, also needed by delta encoding. The performance of bit unpacking is either the same or better with the new implementation, depending on bit width. Updated the results in bit-packing-benchmark.cc. Testing: - Modified bit packing and unpacking tests to test bit widths up to 64 bits. - Tests covering the additions in BitWriter and BatchedBitReader. Change-Id: Ib9276ca291e3d36db16f63f8caf3e9248a18d85b --- M be/src/benchmarks/bit-packing-benchmark.cc M be/src/util/CMakeLists.txt M be/src/util/bit-packing-test.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/bit-stream-utils.inline.h M be/src/util/rle-encoding.h M be/src/util/rle-test.cc 10 files changed, 430 insertions(+), 209 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/09/13809/13 -- To view, visit http://gerrit.cloudera.org:8080/13809 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ib9276ca291e3d36db16f63f8caf3e9248a18d85b Gerrit-Change-Number: 13809 Gerrit-PatchSet: 13 Gerrit-Owner: Daniel Becker Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Zoltan Borok-Nagy
[Impala-ASF-CR] IMPALA-8710: Increase allowed bit width to 64 for bit packing
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13809 ) Change subject: IMPALA-8710: Increase allowed bit width to 64 for bit packing .. Patch Set 11: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/3958/ : 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/13809 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ib9276ca291e3d36db16f63f8caf3e9248a18d85b Gerrit-Change-Number: 13809 Gerrit-PatchSet: 11 Gerrit-Owner: Daniel Becker Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Tue, 23 Jul 2019 08:07:14 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8710: Increase allowed bit width to 64 for bit packing
Daniel Becker has uploaded a new patch set (#11). ( http://gerrit.cloudera.org:8080/13809 ) Change subject: IMPALA-8710: Increase allowed bit width to 64 for bit packing .. IMPALA-8710: Increase allowed bit width to 64 for bit packing Fixed the buffer overflow that the previous attempt (commit b1cbf9e6b786132e86699cbb1e472ec98499bb11, https://gerrit.cloudera.org/#/c/13737) introduced. Compared to that change, only bit-packing.inline.h is different. The tests went into the buffer overflow path but it only produced an error in the ASAN builds. Increasing the allowed bit width for bit packing and bit unpacking to 64 bits. This will be needed to support the Parquet delta encoding. Added new methods to BitWriter and BatchedBitReader handling Uleb and ZigZag integers for 64 bits, also needed by delta encoding. The performance of bit unpacking is either the same or better with the new implementation, depending on bit width. Updated the results in bit-packing-benchmark.cc. Testing: - Modified bit packing and unpacking tests to test bit widths up to 64 bits. - Tests covering the additions in BitWriter and BatchedBitReader. Change-Id: Ib9276ca291e3d36db16f63f8caf3e9248a18d85b --- M be/src/benchmarks/bit-packing-benchmark.cc M be/src/util/CMakeLists.txt M be/src/util/bit-packing-test.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/bit-stream-utils.inline.h M be/src/util/rle-encoding.h M be/src/util/rle-test.cc 10 files changed, 430 insertions(+), 209 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/09/13809/11 -- To view, visit http://gerrit.cloudera.org:8080/13809 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ib9276ca291e3d36db16f63f8caf3e9248a18d85b Gerrit-Change-Number: 13809 Gerrit-PatchSet: 11 Gerrit-Owner: Daniel Becker Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Zoltan Borok-Nagy
[Impala-ASF-CR] IMPALA-8710: Increase allowed bit width to 64 for bit packing
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13809 ) Change subject: IMPALA-8710: Increase allowed bit width to 64 for bit packing .. Patch Set 9: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/3942/ : 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/13809 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ib9276ca291e3d36db16f63f8caf3e9248a18d85b Gerrit-Change-Number: 13809 Gerrit-PatchSet: 9 Gerrit-Owner: Daniel Becker Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Mon, 22 Jul 2019 10:20:14 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8710: Increase allowed bit width to 64 for bit packing
Daniel Becker has uploaded a new patch set (#9). ( http://gerrit.cloudera.org:8080/13809 ) Change subject: IMPALA-8710: Increase allowed bit width to 64 for bit packing .. IMPALA-8710: Increase allowed bit width to 64 for bit packing Fixed the buffer overflow that the previous attempt (commit b1cbf9e6b786132e86699cbb1e472ec98499bb11, https://gerrit.cloudera.org/#/c/13737) introduced. Compared to that change, only bit-packing.inline.h is different. The tests went into the buffer overflow path but it only produced an error in the ASAN builds. Increasing the allowed bit width for bit packing and bit unpacking to 64 bits. This will be needed to support the Parquet delta encoding. Added new methods to BitWriter and BatchedBitReader handling Uleb and ZigZag integers for 64 bits, also needed by delta encoding. The performance of bit unpacking is either the same or better with the new implementation, depending on bit width. Updated the results in bit-packing-benchmark.cc. Testing: - Modified bit packing and unpacking tests to test bit widths up to 64 bits. - Tests covering the additions in BitWriter and BatchedBitReader. Change-Id: Ib9276ca291e3d36db16f63f8caf3e9248a18d85b --- M be/src/benchmarks/bit-packing-benchmark.cc M be/src/util/CMakeLists.txt M be/src/util/bit-packing-test.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/bit-stream-utils.inline.h M be/src/util/rle-encoding.h M be/src/util/rle-test.cc 10 files changed, 430 insertions(+), 209 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/09/13809/9 -- To view, visit http://gerrit.cloudera.org:8080/13809 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ib9276ca291e3d36db16f63f8caf3e9248a18d85b Gerrit-Change-Number: 13809 Gerrit-PatchSet: 9 Gerrit-Owner: Daniel Becker Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Zoltan Borok-Nagy
[Impala-ASF-CR] IMPALA-8710: Increase allowed bit width to 64 for bit packing
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13809 ) Change subject: IMPALA-8710: Increase allowed bit width to 64 for bit packing .. Patch Set 8: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/3917/ : 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/13809 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ib9276ca291e3d36db16f63f8caf3e9248a18d85b Gerrit-Change-Number: 13809 Gerrit-PatchSet: 8 Gerrit-Owner: Daniel Becker Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Fri, 19 Jul 2019 09:24:01 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8710: Increase allowed bit width to 64 for bit packing
Daniel Becker has uploaded a new patch set (#8). ( http://gerrit.cloudera.org:8080/13809 ) Change subject: IMPALA-8710: Increase allowed bit width to 64 for bit packing .. IMPALA-8710: Increase allowed bit width to 64 for bit packing Fixed the buffer overflow that the previous attempt (commit b1cbf9e6b786132e86699cbb1e472ec98499bb11, https://gerrit.cloudera.org/#/c/13737) introduced. Compared to that change, only bit-packing.inline.h is different. The tests went into the buffer overflow path but it only produced an error in the ASAN builds. Increasing the allowed bit width for bit packing and bit unpacking to 64 bits. This will be needed to support the Parquet delta encoding. Added new methods to BitWriter and BatchedBitReader handling Uleb and ZigZag integers for 64 bits, also needed by delta encoding. The performance of bit unpacking is either the same or better with the new implementation, depending on bit width. Updated the results in bit-packing-benchmark.cc. Testing: - Modified bit packing and unpacking tests to test bit widths up to 64 bits. - Tests covering the additions in BitWriter and BatchedBitReader. Change-Id: Ib9276ca291e3d36db16f63f8caf3e9248a18d85b --- M be/src/benchmarks/bit-packing-benchmark.cc M be/src/util/CMakeLists.txt M be/src/util/bit-packing-test.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/bit-stream-utils.inline.h M be/src/util/rle-encoding.h M be/src/util/rle-test.cc 10 files changed, 430 insertions(+), 209 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/09/13809/8 -- To view, visit http://gerrit.cloudera.org:8080/13809 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ib9276ca291e3d36db16f63f8caf3e9248a18d85b Gerrit-Change-Number: 13809 Gerrit-PatchSet: 8 Gerrit-Owner: Daniel Becker Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Zoltan Borok-Nagy
[Impala-ASF-CR] IMPALA-8710: Increase allowed bit width to 64 for bit packing
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13809 ) Change subject: IMPALA-8710: Increase allowed bit width to 64 for bit packing .. Patch Set 7: Verified-1 Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/4613/ -- To view, visit http://gerrit.cloudera.org:8080/13809 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ib9276ca291e3d36db16f63f8caf3e9248a18d85b Gerrit-Change-Number: 13809 Gerrit-PatchSet: 7 Gerrit-Owner: Daniel Becker Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Thu, 18 Jul 2019 22:10:52 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8710: Increase allowed bit width to 64 for bit packing
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13809 ) Change subject: IMPALA-8710: Increase allowed bit width to 64 for bit packing .. Patch Set 7: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/13809 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ib9276ca291e3d36db16f63f8caf3e9248a18d85b Gerrit-Change-Number: 13809 Gerrit-PatchSet: 7 Gerrit-Owner: Daniel Becker Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Thu, 18 Jul 2019 15:55:34 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8710: Increase allowed bit width to 64 for bit packing
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13809 ) Change subject: IMPALA-8710: Increase allowed bit width to 64 for bit packing .. Patch Set 7: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/4613/ DRY_RUN=false -- To view, visit http://gerrit.cloudera.org:8080/13809 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ib9276ca291e3d36db16f63f8caf3e9248a18d85b Gerrit-Change-Number: 13809 Gerrit-PatchSet: 7 Gerrit-Owner: Daniel Becker Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Thu, 18 Jul 2019 15:55:35 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8710: Increase allowed bit width to 64 for bit packing
Zoltan Borok-Nagy has posted comments on this change. ( http://gerrit.cloudera.org:8080/13809 ) Change subject: IMPALA-8710: Increase allowed bit width to 64 for bit packing .. Patch Set 6: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/13809 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ib9276ca291e3d36db16f63f8caf3e9248a18d85b Gerrit-Change-Number: 13809 Gerrit-PatchSet: 6 Gerrit-Owner: Daniel Becker Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Thu, 18 Jul 2019 15:54:29 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8710: Increase allowed bit width to 64 for bit packing
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13809 ) Change subject: IMPALA-8710: Increase allowed bit width to 64 for bit packing .. Patch Set 6: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/3896/ : 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/13809 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ib9276ca291e3d36db16f63f8caf3e9248a18d85b Gerrit-Change-Number: 13809 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-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Wed, 17 Jul 2019 13:58:37 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8710: Increase allowed bit width to 64 for bit packing
Zoltan Borok-Nagy has posted comments on this change. ( http://gerrit.cloudera.org:8080/13809 ) Change subject: IMPALA-8710: Increase allowed bit width to 64 for bit packing .. Patch Set 6: Code-Review+1 -- To view, visit http://gerrit.cloudera.org:8080/13809 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ib9276ca291e3d36db16f63f8caf3e9248a18d85b Gerrit-Change-Number: 13809 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-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Wed, 17 Jul 2019 13:23:04 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8710: Increase allowed bit width to 64 for bit packing
Daniel Becker has posted comments on this change. ( http://gerrit.cloudera.org:8080/13809 ) Change subject: IMPALA-8710: Increase allowed bit width to 64 for bit packing .. Patch Set 6: (2 comments) http://gerrit.cloudera.org:8080/#/c/13809/4//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/13809/4//COMMIT_MSG@9 PS4, Line 9: Fixed the buffer overflow that the previous attempt (commit > Instead of the change-id I think it's better to link the commit hash or may I added both. http://gerrit.cloudera.org:8080/#/c/13809/5/be/src/util/bit-packing.inline.h File be/src/util/bit-packing.inline.h: http://gerrit.cloudera.org:8080/#/c/13809/5/be/src/util/bit-packing.inline.h@196 PS5, Line 196: constexpr bool READ_32_BITS = WORDS_TO_READ == 1 > Can we safely read 32 bits when it is not a full batch? We can because BitPacking::UnpackUpTo31Values, which calls this function, DCHECKs the length of the input buffer. -- To view, visit http://gerrit.cloudera.org:8080/13809 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ib9276ca291e3d36db16f63f8caf3e9248a18d85b Gerrit-Change-Number: 13809 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-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Wed, 17 Jul 2019 13:18:25 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8710: Increase allowed bit width to 64 for bit packing
Daniel Becker has uploaded a new patch set (#6). ( http://gerrit.cloudera.org:8080/13809 ) Change subject: IMPALA-8710: Increase allowed bit width to 64 for bit packing .. IMPALA-8710: Increase allowed bit width to 64 for bit packing Fixed the buffer overflow that the previous attempt (commit b1cbf9e6b786132e86699cbb1e472ec98499bb11, https://gerrit.cloudera.org/#/c/13737) introduced. Compared to that change, only bit-packing.inline.h is different. The tests went into the buffer overflow path but it only produced an error in the ASAN builds. Increasing the allowed bit width for bit packing and bit unpacking to 64 bits. This will be needed to support the Parquet delta encoding. Added new methods to BitWriter and BatchedBitReader handling Uleb and ZigZag integers for 64 bits, also needed by delta encoding. The performance of bit unpacking is either the same or better with the new implementation, depending on bit width. Updated the results in bit-packing-benchmark.cc. Testing: - Modified bit packing and unpacking tests to test bit widths up to 64 bits. - Tests covering the additions in BitWriter and BatchedBitReader. Change-Id: Ib9276ca291e3d36db16f63f8caf3e9248a18d85b --- M be/src/benchmarks/bit-packing-benchmark.cc M be/src/util/CMakeLists.txt M be/src/util/bit-packing-test.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/bit-stream-utils.inline.h M be/src/util/rle-encoding.h M be/src/util/rle-test.cc 10 files changed, 430 insertions(+), 209 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/09/13809/6 -- To view, visit http://gerrit.cloudera.org:8080/13809 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ib9276ca291e3d36db16f63f8caf3e9248a18d85b Gerrit-Change-Number: 13809 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-Reviewer: Zoltan Borok-Nagy
[Impala-ASF-CR] IMPALA-8710: Increase allowed bit width to 64 for bit packing
Zoltan Borok-Nagy has posted comments on this change. ( http://gerrit.cloudera.org:8080/13809 ) Change subject: IMPALA-8710: Increase allowed bit width to 64 for bit packing .. Patch Set 5: (1 comment) http://gerrit.cloudera.org:8080/#/c/13809/5/be/src/util/bit-packing.inline.h File be/src/util/bit-packing.inline.h: http://gerrit.cloudera.org:8080/#/c/13809/5/be/src/util/bit-packing.inline.h@196 PS5, Line 196: constexpr bool READ_32_BITS = WORDS_TO_READ == 1 Can we safely read 32 bits when it is not a full batch? -- To view, visit http://gerrit.cloudera.org:8080/13809 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ib9276ca291e3d36db16f63f8caf3e9248a18d85b Gerrit-Change-Number: 13809 Gerrit-PatchSet: 5 Gerrit-Owner: Daniel Becker Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Wed, 17 Jul 2019 12:59:51 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8710: Increase allowed bit width to 64 for bit packing
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13809 ) Change subject: IMPALA-8710: Increase allowed bit width to 64 for bit packing .. Patch Set 5: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/3895/ : 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/13809 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ib9276ca291e3d36db16f63f8caf3e9248a18d85b Gerrit-Change-Number: 13809 Gerrit-PatchSet: 5 Gerrit-Owner: Daniel Becker Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Wed, 17 Jul 2019 12:57:51 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8710: Increase allowed bit width to 64 for bit packing
Zoltan Borok-Nagy has posted comments on this change. ( http://gerrit.cloudera.org:8080/13809 ) Change subject: IMPALA-8710: Increase allowed bit width to 64 for bit packing .. Patch Set 5: Code-Review+1 (1 comment) http://gerrit.cloudera.org:8080/#/c/13809/4//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/13809/4//COMMIT_MSG@9 PS4, Line 9: Fixed the buffer overflow that the previous attempt (Change-Id: > I linked the previous attempt with its change id. Should I also include the Instead of the change-id I think it's better to link the commit hash or maybe the URL to the code review. -- To view, visit http://gerrit.cloudera.org:8080/13809 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ib9276ca291e3d36db16f63f8caf3e9248a18d85b Gerrit-Change-Number: 13809 Gerrit-PatchSet: 5 Gerrit-Owner: Daniel Becker Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Wed, 17 Jul 2019 12:49:29 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8710: Increase allowed bit width to 64 for bit packing
Daniel Becker has uploaded a new patch set (#5). ( http://gerrit.cloudera.org:8080/13809 ) Change subject: IMPALA-8710: Increase allowed bit width to 64 for bit packing .. IMPALA-8710: Increase allowed bit width to 64 for bit packing Fixed the buffer overflow that the previous attempt (Change-Id: I3ff3b387cbe8e41dd78c45411ef54de4afa8) introduced. Compared to that change, only bit-packing.inline.h is different. The tests went into the buffer overflow path but it only produced an error in the ASAN builds. Increasing the allowed bit width for bit packing and bit unpacking to 64 bits. This will be needed to support the Parquet delta encoding. Added new methods to BitWriter and BatchedBitReader handling Uleb and ZigZag integers for 64 bits, also needed by delta encoding. The performance of bit unpacking is either the same or better with the new implementation, depending on bit width. Updated the results in bit-packing-benchmark.cc. Testing: - Modified bit packing and unpacking tests to test bit widths up to 64 bits. - Tests covering the additions in BitWriter and BatchedBitReader. Change-Id: Ib9276ca291e3d36db16f63f8caf3e9248a18d85b --- M be/src/benchmarks/bit-packing-benchmark.cc M be/src/util/CMakeLists.txt M be/src/util/bit-packing-test.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/bit-stream-utils.inline.h M be/src/util/rle-encoding.h M be/src/util/rle-test.cc 10 files changed, 430 insertions(+), 209 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/09/13809/5 -- To view, visit http://gerrit.cloudera.org:8080/13809 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ib9276ca291e3d36db16f63f8caf3e9248a18d85b Gerrit-Change-Number: 13809 Gerrit-PatchSet: 5 Gerrit-Owner: Daniel Becker Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Zoltan Borok-Nagy
[Impala-ASF-CR] IMPALA-8710: Increase allowed bit width to 64 for bit packing
Daniel Becker has posted comments on this change. ( http://gerrit.cloudera.org:8080/13809 ) Change subject: IMPALA-8710: Increase allowed bit width to 64 for bit packing .. Patch Set 5: (1 comment) http://gerrit.cloudera.org:8080/#/c/13809/4//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/13809/4//COMMIT_MSG@9 PS4, Line 9: Fixed the buffer overflow that the previous attempt (Change-Id: > Can you add a link to the previous attempt? I linked the previous attempt with its change id. Should I also include the commit hash? I added the information you requested. -- To view, visit http://gerrit.cloudera.org:8080/13809 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ib9276ca291e3d36db16f63f8caf3e9248a18d85b Gerrit-Change-Number: 13809 Gerrit-PatchSet: 5 Gerrit-Owner: Daniel Becker Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Wed, 17 Jul 2019 12:16:19 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8710: Increase allowed bit width to 64 for bit packing
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13809 ) Change subject: IMPALA-8710: Increase allowed bit width to 64 for bit packing .. Patch Set 3: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/3847/ : 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/13809 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ib9276ca291e3d36db16f63f8caf3e9248a18d85b Gerrit-Change-Number: 13809 Gerrit-PatchSet: 3 Gerrit-Owner: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Tue, 09 Jul 2019 08:21:03 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8710: Increase allowed bit width to 64 for bit packing
Daniel Becker has uploaded a new patch set (#3). ( http://gerrit.cloudera.org:8080/13809 ) Change subject: IMPALA-8710: Increase allowed bit width to 64 for bit packing .. IMPALA-8710: Increase allowed bit width to 64 for bit packing Fixed the buffer overflow that the previous attempt introduced. Increasing the allowed bit width for bit packing and bit unpacking to 64 bits. This will be needed to support the Parquet delta encoding. Added new methods to BitWriter and BatchedBitReader handling Uleb and ZigZag integers for 64 bits, also needed by delta encoding. The performance of bit unpacking is either the same or better with the new implementation, depending on bit width. Updated the results in bit-packing-benchmark.cc. Testing: - Modified bit packing and unpacking tests to test bit widths up to 64 bits. - Tests covering the additions in BitWriter and BatchedBitReader. Change-Id: Ib9276ca291e3d36db16f63f8caf3e9248a18d85b --- M be/src/benchmarks/bit-packing-benchmark.cc M be/src/util/CMakeLists.txt M be/src/util/bit-packing-test.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/bit-stream-utils.inline.h M be/src/util/rle-encoding.h M be/src/util/rle-test.cc 10 files changed, 430 insertions(+), 209 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/09/13809/3 -- To view, visit http://gerrit.cloudera.org:8080/13809 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ib9276ca291e3d36db16f63f8caf3e9248a18d85b Gerrit-Change-Number: 13809 Gerrit-PatchSet: 3 Gerrit-Owner: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins
[Impala-ASF-CR] IMPALA-8710: Increase allowed bit width to 64 for bit packing
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13809 ) Change subject: IMPALA-8710: Increase allowed bit width to 64 for bit packing .. Patch Set 2: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/3835/ : 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/13809 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ib9276ca291e3d36db16f63f8caf3e9248a18d85b Gerrit-Change-Number: 13809 Gerrit-PatchSet: 2 Gerrit-Owner: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Mon, 08 Jul 2019 13:50:11 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8710: Increase allowed bit width to 64 for bit packing
Daniel Becker has uploaded a new patch set (#2). ( http://gerrit.cloudera.org:8080/13809 ) Change subject: IMPALA-8710: Increase allowed bit width to 64 for bit packing .. IMPALA-8710: Increase allowed bit width to 64 for bit packing Fixed the buffer overflow that the previous attempt introduced. Increasing the allowed bit width for bit packing and bit unpacking to 64 bits. This will be needed to support the Parquet delta encoding. Added new methods to BitWriter and BatchedBitReader handling Uleb and ZigZag integers for 64 bits, also needed by delta encoding. The performance of bit unpacking is either the same or better with the new implementation, depending on bit width. Updated the results in bit-packing-benchmark.cc. Testing: - Modified bit packing and unpacking tests to test bit widths up to 64 bits. - Tests covering the additions in BitWriter and BatchedBitReader. Change-Id: Ib9276ca291e3d36db16f63f8caf3e9248a18d85b --- M be/src/benchmarks/bit-packing-benchmark.cc M be/src/util/CMakeLists.txt M be/src/util/bit-packing-test.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/bit-stream-utils.inline.h M be/src/util/rle-encoding.h M be/src/util/rle-test.cc 10 files changed, 430 insertions(+), 209 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/09/13809/2 -- To view, visit http://gerrit.cloudera.org:8080/13809 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ib9276ca291e3d36db16f63f8caf3e9248a18d85b Gerrit-Change-Number: 13809 Gerrit-PatchSet: 2 Gerrit-Owner: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins
[Impala-ASF-CR] IMPALA-8710: Increase allowed bit width to 64 for bit packing
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13809 ) Change subject: IMPALA-8710: Increase allowed bit width to 64 for bit packing .. Patch Set 1: Build Failed https://jenkins.impala.io/job/gerrit-code-review-checks/3832/ : Initial code review checks failed. See linked job for details on the failure. -- To view, visit http://gerrit.cloudera.org:8080/13809 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ib9276ca291e3d36db16f63f8caf3e9248a18d85b Gerrit-Change-Number: 13809 Gerrit-PatchSet: 1 Gerrit-Owner: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Mon, 08 Jul 2019 10:20:51 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8710: Increase allowed bit width to 64 for bit packing
Daniel Becker has uploaded this change for review. ( http://gerrit.cloudera.org:8080/13809 Change subject: IMPALA-8710: Increase allowed bit width to 64 for bit packing .. IMPALA-8710: Increase allowed bit width to 64 for bit packing Fixed the buffer overflow that the previous attempt introduced. Increasing the allowed bit width for bit packing and bit unpacking to 64 bits. This will be needed to support the Parquet delta encoding. Added new methods to BitWriter and BatchedBitReader handling Uleb and ZigZag integers for 64 bits, also needed by delta encoding. The performance of bit unpacking is either the same or better with the new implementation, depending on bit width. Updated the results in bit-packing-benchmark.cc. Testing: - Modified bit packing and unpacking tests to test bit widths up to 64 bits. - Tests covering the additions in BitWriter and BatchedBitReader. Change-Id: Ib9276ca291e3d36db16f63f8caf3e9248a18d85b --- M be/src/benchmarks/bit-packing-benchmark.cc M be/src/util/bit-packing.inline.h 2 files changed, 188 insertions(+), 171 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/09/13809/1 -- To view, visit http://gerrit.cloudera.org:8080/13809 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: Ib9276ca291e3d36db16f63f8caf3e9248a18d85b Gerrit-Change-Number: 13809 Gerrit-PatchSet: 1 Gerrit-Owner: Daniel Becker
[Impala-ASF-CR] IMPALA-8710: Increase allowed bit width to 64 for bit packing
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/13737 ) Change subject: IMPALA-8710: Increase allowed bit width to 64 for bit packing .. IMPALA-8710: Increase allowed bit width to 64 for bit packing Increasing the allowed bit width for bit packing and bit unpacking to 64 bits. This will be needed to support the Parquet delta encoding. Added new methods to BitWriter and BatchedBitReader handling Uleb and ZigZag integers for 64 bits, also needed by delta encoding. The performance of bit unpacking is either the same or better with the new implementation, depending on bit width. Testing: - Modified bit packing and unpacking tests to test bit widths up to 64 bits. - Tests covering the additions in BitWriter and BatchedBitReader. Change-Id: I3ff3b387cbe8e41dd78c45411ef54de4afa8 Reviewed-on: http://gerrit.cloudera.org:8080/13737 Reviewed-by: Impala Public Jenkins Tested-by: Impala Public Jenkins --- M be/src/benchmarks/bit-packing-benchmark.cc M be/src/util/CMakeLists.txt M be/src/util/bit-packing-test.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/bit-stream-utils.inline.h M be/src/util/rle-encoding.h M be/src/util/rle-test.cc 10 files changed, 414 insertions(+), 204 deletions(-) Approvals: Impala Public Jenkins: Looks good to me, approved; Verified -- To view, visit http://gerrit.cloudera.org:8080/13737 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I3ff3b387cbe8e41dd78c45411ef54de4afa8 Gerrit-Change-Number: 13737 Gerrit-PatchSet: 9 Gerrit-Owner: Daniel Becker Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Zoltan Borok-Nagy
[Impala-ASF-CR] IMPALA-8710: Increase allowed bit width to 64 for bit packing
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13737 ) Change subject: IMPALA-8710: Increase allowed bit width to 64 for bit packing .. Patch Set 8: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/13737 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3ff3b387cbe8e41dd78c45411ef54de4afa8 Gerrit-Change-Number: 13737 Gerrit-PatchSet: 8 Gerrit-Owner: Daniel Becker Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Fri, 05 Jul 2019 01:26:08 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8710: Increase allowed bit width to 64 for bit packing
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13737 ) Change subject: IMPALA-8710: Increase allowed bit width to 64 for bit packing .. Patch Set 8: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/13737 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3ff3b387cbe8e41dd78c45411ef54de4afa8 Gerrit-Change-Number: 13737 Gerrit-PatchSet: 8 Gerrit-Owner: Daniel Becker Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Thu, 04 Jul 2019 19:43:16 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8710: Increase allowed bit width to 64 for bit packing
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13737 ) Change subject: IMPALA-8710: Increase allowed bit width to 64 for bit packing .. Patch Set 8: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/4582/ DRY_RUN=false -- To view, visit http://gerrit.cloudera.org:8080/13737 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3ff3b387cbe8e41dd78c45411ef54de4afa8 Gerrit-Change-Number: 13737 Gerrit-PatchSet: 8 Gerrit-Owner: Daniel Becker Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Thu, 04 Jul 2019 19:43:17 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8710: Increase allowed bit width to 64 for bit packing
Zoltan Borok-Nagy has posted comments on this change. ( http://gerrit.cloudera.org:8080/13737 ) Change subject: IMPALA-8710: Increase allowed bit width to 64 for bit packing .. Patch Set 7: Code-Review+2 Thanks for applying the changes! -- To view, visit http://gerrit.cloudera.org:8080/13737 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3ff3b387cbe8e41dd78c45411ef54de4afa8 Gerrit-Change-Number: 13737 Gerrit-PatchSet: 7 Gerrit-Owner: Daniel Becker Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Thu, 04 Jul 2019 19:42:28 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8710: Increase allowed bit width to 64 for bit packing
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13737 ) Change subject: IMPALA-8710: Increase allowed bit width to 64 for bit packing .. Patch Set 7: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/3824/ : 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/13737 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3ff3b387cbe8e41dd78c45411ef54de4afa8 Gerrit-Change-Number: 13737 Gerrit-PatchSet: 7 Gerrit-Owner: Daniel Becker Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Thu, 04 Jul 2019 08:15:33 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8710: Increase allowed bit width to 64 for bit packing
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13737 ) Change subject: IMPALA-8710: Increase allowed bit width to 64 for bit packing .. Patch Set 6: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/3823/ : 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/13737 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3ff3b387cbe8e41dd78c45411ef54de4afa8 Gerrit-Change-Number: 13737 Gerrit-PatchSet: 6 Gerrit-Owner: Daniel Becker Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Thu, 04 Jul 2019 07:55:55 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8710: Increase allowed bit width to 64 for bit packing
Daniel Becker has posted comments on this change. ( http://gerrit.cloudera.org:8080/13737 ) Change subject: IMPALA-8710: Increase allowed bit width to 64 for bit packing .. Patch Set 7: Rebased on master. -- To view, visit http://gerrit.cloudera.org:8080/13737 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3ff3b387cbe8e41dd78c45411ef54de4afa8 Gerrit-Change-Number: 13737 Gerrit-PatchSet: 7 Gerrit-Owner: Daniel Becker Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Thu, 04 Jul 2019 07:34:43 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8710: Increase allowed bit width to 64 for bit packing
Daniel Becker has uploaded a new patch set (#7). ( http://gerrit.cloudera.org:8080/13737 ) Change subject: IMPALA-8710: Increase allowed bit width to 64 for bit packing .. IMPALA-8710: Increase allowed bit width to 64 for bit packing Increasing the allowed bit width for bit packing and bit unpacking to 64 bits. This will be needed to support the Parquet delta encoding. Added new methods to BitWriter and BatchedBitReader handling Uleb and ZigZag integers for 64 bits, also needed by delta encoding. The performance of bit unpacking is either the same or better with the new implementation, depending on bit width. Testing: - Modified bit packing and unpacking tests to test bit widths up to 64 bits. - Tests covering the additions in BitWriter and BatchedBitReader. Change-Id: I3ff3b387cbe8e41dd78c45411ef54de4afa8 --- M be/src/benchmarks/bit-packing-benchmark.cc M be/src/util/CMakeLists.txt M be/src/util/bit-packing-test.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/bit-stream-utils.inline.h M be/src/util/rle-encoding.h M be/src/util/rle-test.cc 10 files changed, 414 insertions(+), 204 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/37/13737/7 -- To view, visit http://gerrit.cloudera.org:8080/13737 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I3ff3b387cbe8e41dd78c45411ef54de4afa8 Gerrit-Change-Number: 13737 Gerrit-PatchSet: 7 Gerrit-Owner: Daniel Becker Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Zoltan Borok-Nagy
[Impala-ASF-CR] IMPALA-8710: Increase allowed bit width to 64 for bit packing
Daniel Becker has posted comments on this change. ( http://gerrit.cloudera.org:8080/13737 ) Change subject: IMPALA-8710: Increase allowed bit width to 64 for bit packing .. Patch Set 6: (3 comments) http://gerrit.cloudera.org:8080/#/c/13737/5/be/src/util/bit-packing.inline.h File be/src/util/bit-packing.inline.h: http://gerrit.cloudera.org:8080/#/c/13737/5/be/src/util/bit-packing.inline.h@172 PS5, Line 172: constexpr int WORDS_TO_READ = LAST_WORD_IDX - FIRST_WORD_IDX; > nit: You don't use abbreviations at other places, so at first it reads like Done http://gerrit.cloudera.org:8080/#/c/13737/5/be/src/util/bit-packing.inline.h@179 PS5, Line 179: // Avoid reading past the end of the buffer. > nit: This is about not to read past the end of the buffer, right? Can you a Done http://gerrit.cloudera.org:8080/#/c/13737/5/be/src/util/bit-stream-utils-test.cc File be/src/util/bit-stream-utils-test.cc: http://gerrit.cloudera.org:8080/#/c/13737/5/be/src/util/bit-stream-utils-test.cc@158 PS5, Line 158: ~0U > very nit: sometimes in the code there is only L or UL, and sometimes LL, I Done -- To view, visit http://gerrit.cloudera.org:8080/13737 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3ff3b387cbe8e41dd78c45411ef54de4afa8 Gerrit-Change-Number: 13737 Gerrit-PatchSet: 6 Gerrit-Owner: Daniel Becker Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Thu, 04 Jul 2019 07:15:29 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8710: Increase allowed bit width to 64 for bit packing
Daniel Becker has uploaded a new patch set (#6). ( http://gerrit.cloudera.org:8080/13737 ) Change subject: IMPALA-8710: Increase allowed bit width to 64 for bit packing .. IMPALA-8710: Increase allowed bit width to 64 for bit packing Increasing the allowed bit width for bit packing and bit unpacking to 64 bits. This will be needed to support the Parquet delta encoding. Added new methods to BitWriter and BatchedBitReader handling Uleb and ZigZag integers for 64 bits, also needed by delta encoding. The performance of bit unpacking is either the same or better with the new implementation, depending on bit width. Testing: - Modified bit packing and unpacking tests to test bit widths up to 64 bits. - Tests covering the additions in BitWriter and BatchedBitReader. Change-Id: I3ff3b387cbe8e41dd78c45411ef54de4afa8 --- M be/src/benchmarks/bit-packing-benchmark.cc M be/src/util/CMakeLists.txt M be/src/util/bit-packing-test.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/bit-stream-utils.inline.h M be/src/util/rle-encoding.h M be/src/util/rle-test.cc 10 files changed, 414 insertions(+), 204 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/37/13737/6 -- To view, visit http://gerrit.cloudera.org:8080/13737 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I3ff3b387cbe8e41dd78c45411ef54de4afa8 Gerrit-Change-Number: 13737 Gerrit-PatchSet: 6 Gerrit-Owner: Daniel Becker Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Zoltan Borok-Nagy
[Impala-ASF-CR] IMPALA-8710: Increase allowed bit width to 64 for bit packing
Zoltan Borok-Nagy has posted comments on this change. ( http://gerrit.cloudera.org:8080/13737 ) Change subject: IMPALA-8710: Increase allowed bit width to 64 for bit packing .. Patch Set 5: Code-Review+1 (4 comments) http://gerrit.cloudera.org:8080/#/c/13737/5/be/src/util/bit-packing.inline.h File be/src/util/bit-packing.inline.h: http://gerrit.cloudera.org:8080/#/c/13737/5/be/src/util/bit-packing.inline.h@172 PS5, Line 172: constexpr int NO_WORDS = LAST_WORD_IDX - FIRST_WORD_IDX; nit: You don't use abbreviations at other places, so at first it reads like "no words". Maybe WORDS_TO_READ? http://gerrit.cloudera.org:8080/#/c/13737/5/be/src/util/bit-packing.inline.h@179 PS5, Line 179: constexpr bool CAN_READ_64 = FIRST_BIT_IDX - FIRST_BIT_OFFSET + 64 <= BIT_WIDTH * 32; nit: This is about not to read past the end of the buffer, right? Can you add comment for it? http://gerrit.cloudera.org:8080/#/c/13737/5/be/src/util/bit-stream-utils-test.cc File be/src/util/bit-stream-utils-test.cc: http://gerrit.cloudera.org:8080/#/c/13737/5/be/src/util/bit-stream-utils-test.cc@158 PS5, Line 158: int64_t nit: uint64_t? http://gerrit.cloudera.org:8080/#/c/13737/5/be/src/util/bit-stream-utils-test.cc@158 PS5, Line 158: 0LL very nit: sometimes in the code there is only L or UL, and sometimes LL, I think UL is the choice for most places. It doesn't really matter, but makes the reader to think for a sec why this specific suffix is used here. -- To view, visit http://gerrit.cloudera.org:8080/13737 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3ff3b387cbe8e41dd78c45411ef54de4afa8 Gerrit-Change-Number: 13737 Gerrit-PatchSet: 5 Gerrit-Owner: Daniel Becker Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Tue, 02 Jul 2019 15:48:48 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8710: Increase allowed bit width to 64 for bit packing
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13737 ) Change subject: IMPALA-8710: Increase allowed bit width to 64 for bit packing .. Patch Set 5: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/3781/ : 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/13737 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3ff3b387cbe8e41dd78c45411ef54de4afa8 Gerrit-Change-Number: 13737 Gerrit-PatchSet: 5 Gerrit-Owner: Daniel Becker Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Fri, 28 Jun 2019 15:11:47 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8710: Increase allowed bit width to 64 for bit packing
Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/13737 ) Change subject: IMPALA-8710: Increase allowed bit width to 64 for bit packing .. Patch Set 5: Code-Review+1 -- To view, visit http://gerrit.cloudera.org:8080/13737 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3ff3b387cbe8e41dd78c45411ef54de4afa8 Gerrit-Change-Number: 13737 Gerrit-PatchSet: 5 Gerrit-Owner: Daniel Becker Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Fri, 28 Jun 2019 14:41:09 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8710: Increase allowed bit width to 64 for bit packing
Daniel Becker has posted comments on this change. ( http://gerrit.cloudera.org:8080/13737 ) Change subject: IMPALA-8710: Increase allowed bit width to 64 for bit packing .. Patch Set 5: Removed trailing whitespace. -- To view, visit http://gerrit.cloudera.org:8080/13737 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3ff3b387cbe8e41dd78c45411ef54de4afa8 Gerrit-Change-Number: 13737 Gerrit-PatchSet: 5 Gerrit-Owner: Daniel Becker Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Fri, 28 Jun 2019 14:31:28 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8710: Increase allowed bit width to 64 for bit packing
Daniel Becker has uploaded a new patch set (#5). ( http://gerrit.cloudera.org:8080/13737 ) Change subject: IMPALA-8710: Increase allowed bit width to 64 for bit packing .. IMPALA-8710: Increase allowed bit width to 64 for bit packing Increasing the allowed bit width for bit packing and bit unpacking to 64 bits. This will be needed to support the Parquet delta encoding. Added new methods to BitWriter and BatchedBitReader handling Uleb and ZigZag integers for 64 bits, also needed by delta encoding. The performance of bit unpacking is either the same or better with the new implementation, depending on bit width. Testing: - Modified bit packing and unpacking tests to test bit widths up to 64 bits. - Tests covering the additions in BitWriter and BatchedBitReader. Change-Id: I3ff3b387cbe8e41dd78c45411ef54de4afa8 --- M be/src/benchmarks/bit-packing-benchmark.cc M be/src/util/CMakeLists.txt M be/src/util/bit-packing-test.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/bit-stream-utils.inline.h M be/src/util/rle-encoding.h M be/src/util/rle-test.cc 10 files changed, 413 insertions(+), 204 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/37/13737/5 -- To view, visit http://gerrit.cloudera.org:8080/13737 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I3ff3b387cbe8e41dd78c45411ef54de4afa8 Gerrit-Change-Number: 13737 Gerrit-PatchSet: 5 Gerrit-Owner: Daniel Becker Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins
[Impala-ASF-CR] IMPALA-8710: Increase allowed bit width to 64 for bit packing
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13737 ) Change subject: IMPALA-8710: Increase allowed bit width to 64 for bit packing .. Patch Set 4: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/3780/ : 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/13737 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3ff3b387cbe8e41dd78c45411ef54de4afa8 Gerrit-Change-Number: 13737 Gerrit-PatchSet: 4 Gerrit-Owner: Daniel Becker Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Fri, 28 Jun 2019 14:10:43 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8710: Increase allowed bit width to 64 for bit packing
Daniel Becker has posted comments on this change. ( http://gerrit.cloudera.org:8080/13737 ) Change subject: IMPALA-8710: Increase allowed bit width to 64 for bit packing .. Patch Set 4: Updated the results in bit-packing-benchmark.cc -- To view, visit http://gerrit.cloudera.org:8080/13737 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3ff3b387cbe8e41dd78c45411ef54de4afa8 Gerrit-Change-Number: 13737 Gerrit-PatchSet: 4 Gerrit-Owner: Daniel Becker Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Fri, 28 Jun 2019 13:30:26 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8710: Increase allowed bit width to 64 for bit packing
Daniel Becker has uploaded a new patch set (#4). ( http://gerrit.cloudera.org:8080/13737 ) Change subject: IMPALA-8710: Increase allowed bit width to 64 for bit packing .. IMPALA-8710: Increase allowed bit width to 64 for bit packing Increasing the allowed bit width for bit packing and bit unpacking to 64 bits. This will be needed to support the Parquet delta encoding. Added new methods to BitWriter and BatchedBitReader handling Uleb and ZigZag integers for 64 bits, also needed by delta encoding. The performance of bit unpacking is either the same or better with the new implementation, depending on bit width. Testing: - Modified bit packing and unpacking tests to test bit widths up to 64 bits. - Tests covering the additions in BitWriter and BatchedBitReader. Change-Id: I3ff3b387cbe8e41dd78c45411ef54de4afa8 --- M be/src/benchmarks/bit-packing-benchmark.cc M be/src/util/CMakeLists.txt M be/src/util/bit-packing-test.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/bit-stream-utils.inline.h M be/src/util/rle-encoding.h M be/src/util/rle-test.cc 10 files changed, 445 insertions(+), 236 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/37/13737/4 -- To view, visit http://gerrit.cloudera.org:8080/13737 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I3ff3b387cbe8e41dd78c45411ef54de4afa8 Gerrit-Change-Number: 13737 Gerrit-PatchSet: 4 Gerrit-Owner: Daniel Becker Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins
[Impala-ASF-CR] IMPALA-8710: Increase allowed bit width to 64 for bit packing
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13737 ) Change subject: IMPALA-8710: Increase allowed bit width to 64 for bit packing .. Patch Set 3: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/3779/ : 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/13737 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3ff3b387cbe8e41dd78c45411ef54de4afa8 Gerrit-Change-Number: 13737 Gerrit-PatchSet: 3 Gerrit-Owner: Daniel Becker Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Fri, 28 Jun 2019 12:52:08 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8710: Increase allowed bit width to 64 for bit packing
Daniel Becker has posted comments on this change. (
http://gerrit.cloudera.org:8080/13737 )
Change subject: IMPALA-8710: Increase allowed bit width to 64 for bit packing
..
Patch Set 3:
(9 comments)
http://gerrit.cloudera.org:8080/#/c/13737/2//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/13737/2//COMMIT_MSG@14
PS2, Line 14:
> Performance effects could be mentioned +
Done
http://gerrit.cloudera.org:8080/#/c/13737/2//COMMIT_MSG@16
PS2, Line 16:
> nit: wrap at 72
Done
http://gerrit.cloudera.org:8080/#/c/13737/2/be/src/util/bit-packing.inline.h
File be/src/util/bit-packing.inline.h:
http://gerrit.cloudera.org:8080/#/c/13737/2/be/src/util/bit-packing.inline.h@177
PS2, Line 177: const uint32_t* const in = re
> I would move this to the very beginning to make it clear that the rest of t
Done
http://gerrit.cloudera.org:8080/#/c/13737/2/be/src/util/bit-packing.inline.h@181
PS2, Line 181: p
> nit: extra space
Done
http://gerrit.cloudera.org:8080/#/c/13737/2/be/src/util/bit-packing.inline.h@184
PS2, Line 184: == 1
> Can you move this to a constexpr to make it clear that this happens at comp
Done
http://gerrit.cloudera.org:8080/#/c/13737/2/be/src/util/bit-stream-utils-test.cc
File be/src/util/bit-stream-utils-test.cc:
http://gerrit.cloudera.org:8080/#/c/13737/2/be/src/util/bit-stream-utils-test.cc@294
PS2, Line 294: TestZigZagEncode(1629267541664, {0xc0, 0xfa, 0xcd,
0xfe, 0xea, 0x5e});
> nit:extra line
Done
http://gerrit.cloudera.org:8080/#/c/13737/2/be/src/util/bit-stream-utils-test.cc@315
PS2, Line 315: TestZigZagDecode(-9223372036854775808U, // Most
negative int64_t.
> nit:extra line
Done
http://gerrit.cloudera.org:8080/#/c/13737/2/be/src/util/bit-stream-utils-test.cc@320
PS2, Line 320: TEST(VLQInt, TestMaxVlqByteLen) {
> nit:extra line
Done
http://gerrit.cloudera.org:8080/#/c/13737/2/be/src/util/bit-stream-utils.inline.h
File be/src/util/bit-stream-utils.inline.h:
http://gerrit.cloudera.org:8080/#/c/13737/2/be/src/util/bit-stream-utils.inline.h@44
PS2, Line 44: un
> Can you expand the abbreviation? It took me a few seconds to understand it.
Done
--
To view, visit http://gerrit.cloudera.org:8080/13737
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3ff3b387cbe8e41dd78c45411ef54de4afa8
Gerrit-Change-Number: 13737
Gerrit-PatchSet: 3
Gerrit-Owner: Daniel Becker
Gerrit-Reviewer: Csaba Ringhofer
Gerrit-Reviewer: Daniel Becker
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Comment-Date: Fri, 28 Jun 2019 12:11:36 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8710: Increase allowed bit width to 64 for bit packing
Daniel Becker has uploaded a new patch set (#3). ( http://gerrit.cloudera.org:8080/13737 ) Change subject: IMPALA-8710: Increase allowed bit width to 64 for bit packing .. IMPALA-8710: Increase allowed bit width to 64 for bit packing Increasing the allowed bit width for bit packing and bit unpacking to 64 bits. This will be needed to support the Parquet delta encoding. Added new methods to BitWriter and BatchedBitReader handling Uleb and ZigZag integers for 64 bits, also needed by delta encoding. The performance of bit unpacking is either the same or better with the new implementation, depending on bit width. Testing: - Modified bit packing and unpacking tests to test bit widths up to 64 bits. - Tests covering the additions in BitWriter and BatchedBitReader. Change-Id: I3ff3b387cbe8e41dd78c45411ef54de4afa8 --- M be/src/util/CMakeLists.txt M be/src/util/bit-packing-test.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/bit-stream-utils.inline.h M be/src/util/rle-encoding.h M be/src/util/rle-test.cc 9 files changed, 314 insertions(+), 105 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/37/13737/3 -- To view, visit http://gerrit.cloudera.org:8080/13737 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I3ff3b387cbe8e41dd78c45411ef54de4afa8 Gerrit-Change-Number: 13737 Gerrit-PatchSet: 3 Gerrit-Owner: Daniel Becker Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins
[Impala-ASF-CR] IMPALA-8710: Increase allowed bit width to 64 for bit packing
Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/13737 ) Change subject: IMPALA-8710: Increase allowed bit width to 64 for bit packing .. Patch Set 2: Code-Review+1 (10 comments) I found only nits. http://gerrit.cloudera.org:8080/#/c/13737/2//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/13737/2//COMMIT_MSG@14 PS2, Line 14: Performance effects could be mentioned + https://gerrit.cloudera.org/#/c/12621/ as it contains the benchmarks that were used to verify that there is no performance regression. http://gerrit.cloudera.org:8080/#/c/13737/2//COMMIT_MSG@16 PS2, Line 16: 64 bits. nit: wrap at 72 http://gerrit.cloudera.org:8080/#/c/13737/2/be/src/util/bit-packing.inline.h File be/src/util/bit-packing.inline.h: http://gerrit.cloudera.org:8080/#/c/13737/2/be/src/util/bit-packing.inline.h@177 PS2, Line 177: if (BIT_WIDTH == 0) return 0; I would move this to the very beginning to make it clear that the rest of the logic does not have to handle this case. http://gerrit.cloudera.org:8080/#/c/13737/2/be/src/util/bit-packing.inline.h@181 PS2, Line 181: nit: extra space http://gerrit.cloudera.org:8080/#/c/13737/2/be/src/util/bit-packing.inline.h@184 PS2, Line 184: BitUtil::IsPowerOf2(BIT_WIDTH) Can you move this to a constexpr to make it clear that this happens at compile time? http://gerrit.cloudera.org:8080/#/c/13737/2/be/src/util/bit-stream-utils-test.cc File be/src/util/bit-stream-utils-test.cc: http://gerrit.cloudera.org:8080/#/c/13737/2/be/src/util/bit-stream-utils-test.cc@294 PS2, Line 294: nit:extra line http://gerrit.cloudera.org:8080/#/c/13737/2/be/src/util/bit-stream-utils-test.cc@315 PS2, Line 315: nit:extra line http://gerrit.cloudera.org:8080/#/c/13737/2/be/src/util/bit-stream-utils-test.cc@320 PS2, Line 320: nit:extra line http://gerrit.cloudera.org:8080/#/c/13737/2/be/src/util/bit-stream-utils.inline.h File be/src/util/bit-stream-utils.inline.h: http://gerrit.cloudera.org:8080/#/c/13737/2/be/src/util/bit-stream-utils.inline.h@44 PS2, Line 44: UB Can you expand the abbreviation? It took me a few seconds to understand it. http://gerrit.cloudera.org:8080/#/c/13737/2/be/src/util/bit-stream-utils.inline.h@196 PS2, Line 196: UB Same as line 44. -- To view, visit http://gerrit.cloudera.org:8080/13737 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3ff3b387cbe8e41dd78c45411ef54de4afa8 Gerrit-Change-Number: 13737 Gerrit-PatchSet: 2 Gerrit-Owner: Daniel Becker Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Fri, 28 Jun 2019 11:59:55 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8710: Increase allowed bit width to 64 for bit packing
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13737 ) Change subject: IMPALA-8710: Increase allowed bit width to 64 for bit packing .. Patch Set 2: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/3778/ : 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/13737 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3ff3b387cbe8e41dd78c45411ef54de4afa8 Gerrit-Change-Number: 13737 Gerrit-PatchSet: 2 Gerrit-Owner: Daniel Becker Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Fri, 28 Jun 2019 11:16:34 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8710: Increase allowed bit width to 64 for bit packing
Daniel Becker has uploaded a new patch set (#2). ( http://gerrit.cloudera.org:8080/13737 ) Change subject: IMPALA-8710: Increase allowed bit width to 64 for bit packing .. IMPALA-8710: Increase allowed bit width to 64 for bit packing Increasing the allowed bit width for bit packing and bit unpacking to 64 bits. This will be needed to support the Parquet delta encoding. Added new methods to BitWriter and BatchedBitReader handling Uleb and ZigZag integers for 64 bits, also needed by delta encoding. Testing: - Modified bit packing and unpacking tests to test bit widths up to 64 bits. - Tests covering the additions in BitWriter and BatchedBitReader. Change-Id: I3ff3b387cbe8e41dd78c45411ef54de4afa8 --- M be/src/util/CMakeLists.txt M be/src/util/bit-packing-test.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/bit-stream-utils.inline.h M be/src/util/rle-encoding.h M be/src/util/rle-test.cc 9 files changed, 314 insertions(+), 105 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/37/13737/2 -- To view, visit http://gerrit.cloudera.org:8080/13737 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I3ff3b387cbe8e41dd78c45411ef54de4afa8 Gerrit-Change-Number: 13737 Gerrit-PatchSet: 2 Gerrit-Owner: Daniel Becker Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins
[Impala-ASF-CR] IMPALA-8710: Increase allowed bit width to 64 for bit packing
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13737 ) Change subject: IMPALA-8710: Increase allowed bit width to 64 for bit packing .. Patch Set 1: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/3753/ : 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/13737 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3ff3b387cbe8e41dd78c45411ef54de4afa8 Gerrit-Change-Number: 13737 Gerrit-PatchSet: 1 Gerrit-Owner: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Wed, 26 Jun 2019 17:17:17 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8710: Increase allowed bit width to 64 for bit packing
Daniel Becker has uploaded this change for review. ( http://gerrit.cloudera.org:8080/13737 Change subject: IMPALA-8710: Increase allowed bit width to 64 for bit packing .. IMPALA-8710: Increase allowed bit width to 64 for bit packing Increasing the allowed bit width for bit packing and bit unpacking to 64 bits. This will be needed to support the Parquet delta encoding. Added new methods to BitWriter and BatchedBitReader handling Uleb and ZigZag integers for 64 bits, also needed by delta encoding. Testing: - Modified bit packing and unpacking tests to test bit widths up to 64 bits. - Tests covering the additions in BitWriter and BatchedBitReader. Change-Id: I3ff3b387cbe8e41dd78c45411ef54de4afa8 --- M be/src/util/CMakeLists.txt M be/src/util/bit-packing-test.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/bit-stream-utils.inline.h M be/src/util/rle-encoding.h M be/src/util/rle-test.cc 9 files changed, 307 insertions(+), 105 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/37/13737/1 -- To view, visit http://gerrit.cloudera.org:8080/13737 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I3ff3b387cbe8e41dd78c45411ef54de4afa8 Gerrit-Change-Number: 13737 Gerrit-PatchSet: 1 Gerrit-Owner: Daniel Becker
