You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@impala.apache.org by "Tim Armstrong (Code Review)" <ge...@cloudera.org> on 2016/09/01 21:09:19 UTC
[Impala-ASF-CR] IMPALA-4058: benchmark byteswap on misaligned memory
Tim Armstrong has uploaded a new change for review.
http://gerrit.cloudera.org:8080/4290
Change subject: IMPALA-4058: benchmark byteswap on misaligned memory
......................................................................
IMPALA-4058: benchmark byteswap on misaligned memory
Extend the benchmark to measure performance when memory is misaligned.
This is a common case when decoding parquet.
Change-Id: Ia9dfcca718b2456488d0fb4407d3817c796bfad0
---
M be/src/benchmarks/bswap-benchmark.cc
1 file changed, 54 insertions(+), 26 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/90/4290/1
--
To view, visit http://gerrit.cloudera.org:8080/4290
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ia9dfcca718b2456488d0fb4407d3817c796bfad0
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
[Impala-ASF-CR] IMPALA-4058: benchmark byteswap on misaligned memory
Posted by "Internal Jenkins (Code Review)" <ge...@cloudera.org>.
Internal Jenkins has posted comments on this change.
Change subject: IMPALA-4058: benchmark byteswap on misaligned memory
......................................................................
Patch Set 5: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/4290
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia9dfcca718b2456488d0fb4407d3817c796bfad0
Gerrit-PatchSet: 5
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Internal Jenkins
Gerrit-Reviewer: Jim Apple <jb...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4058: benchmark byteswap on misaligned memory
Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Hello Jim Apple,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/4290
to look at the new patch set (#4).
Change subject: IMPALA-4058: benchmark byteswap on misaligned memory
......................................................................
IMPALA-4058: benchmark byteswap on misaligned memory
Extend the benchmark to measure performance when memory is misaligned.
This is a common case when decoding parquet.
Change-Id: Ia9dfcca718b2456488d0fb4407d3817c796bfad0
---
M be/src/benchmarks/bswap-benchmark.cc
M be/src/common/names.h
2 files changed, 75 insertions(+), 28 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/90/4290/4
--
To view, visit http://gerrit.cloudera.org:8080/4290
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ia9dfcca718b2456488d0fb4407d3817c796bfad0
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Jim Apple <jb...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
[Impala-ASF-CR] IMPALA-4058: benchmark byteswap on misaligned memory
Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Tim Armstrong has posted comments on this change.
Change subject: IMPALA-4058: benchmark byteswap on misaligned memory
......................................................................
Patch Set 3:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/4290/3/be/src/benchmarks/bswap-benchmark.cc
File be/src/benchmarks/bswap-benchmark.cc:
Line 139: uint8_t* outbuffer =
> You could sprinkle some consts in here, too.
Done
Line 158: free(inbuffer);
> This might be RAIIable with unique_ptr custom deleters + auto.
Thanks, I haven't used them before but they seem generally useful.
--
To view, visit http://gerrit.cloudera.org:8080/4290
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia9dfcca718b2456488d0fb4407d3817c796bfad0
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Jim Apple <jb...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4058: benchmark byteswap on misaligned memory
Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Tim Armstrong has posted comments on this change.
Change subject: IMPALA-4058: benchmark byteswap on misaligned memory
......................................................................
Patch Set 4: Code-Review+2
Carry +2
--
To view, visit http://gerrit.cloudera.org:8080/4290
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia9dfcca718b2456488d0fb4407d3817c796bfad0
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Jim Apple <jb...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4058: benchmark byteswap on misaligned memory
Posted by "Jim Apple (Code Review)" <ge...@cloudera.org>.
Jim Apple has posted comments on this change.
Change subject: IMPALA-4058: benchmark byteswap on misaligned memory
......................................................................
Patch Set 4: Code-Review+2
(1 comment)
http://gerrit.cloudera.org:8080/#/c/4290/4/be/src/benchmarks/bswap-benchmark.cc
File be/src/benchmarks/bswap-benchmark.cc:
Line 20: #include <iostream>
nit: https://google.github.io/styleguide/cppguide.html#Names_and_Order_of_Includes
--
To view, visit http://gerrit.cloudera.org:8080/4290
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia9dfcca718b2456488d0fb4407d3817c796bfad0
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Jim Apple <jb...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4058: benchmark byteswap on misaligned memory
Posted by "Jim Apple (Code Review)" <ge...@cloudera.org>.
Jim Apple has posted comments on this change.
Change subject: IMPALA-4058: benchmark byteswap on misaligned memory
......................................................................
Patch Set 3: Code-Review+2
(2 comments)
Just some nits; feel free to skip
http://gerrit.cloudera.org:8080/#/c/4290/3/be/src/benchmarks/bswap-benchmark.cc
File be/src/benchmarks/bswap-benchmark.cc:
Line 139: uint8_t* outbuffer =
You could sprinkle some consts in here, too.
Line 158: free(inbuffer);
This might be RAIIable with unique_ptr custom deleters + auto.
--
To view, visit http://gerrit.cloudera.org:8080/4290
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia9dfcca718b2456488d0fb4407d3817c796bfad0
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Jim Apple <jb...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4058: benchmark byteswap on misaligned memory
Posted by "Jim Apple (Code Review)" <ge...@cloudera.org>.
Jim Apple has posted comments on this change.
Change subject: IMPALA-4058: benchmark byteswap on misaligned memory
......................................................................
Patch Set 3:
> Will wait until https://gerrit.cloudera.org/#/c/4205/6 goes in.
It is in.
--
To view, visit http://gerrit.cloudera.org:8080/4290
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia9dfcca718b2456488d0fb4407d3817c796bfad0
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Jim Apple <jb...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4058: benchmark byteswap on misaligned memory
Posted by "Internal Jenkins (Code Review)" <ge...@cloudera.org>.
Internal Jenkins has submitted this change and it was merged.
Change subject: IMPALA-4058: benchmark byteswap on misaligned memory
......................................................................
IMPALA-4058: benchmark byteswap on misaligned memory
Extend the benchmark to measure performance when memory is misaligned.
This is a common case when decoding parquet.
Change-Id: Ia9dfcca718b2456488d0fb4407d3817c796bfad0
Reviewed-on: http://gerrit.cloudera.org:8080/4290
Reviewed-by: Tim Armstrong <ta...@cloudera.com>
Tested-by: Internal Jenkins
---
M be/src/benchmarks/bswap-benchmark.cc
M be/src/common/names.h
2 files changed, 77 insertions(+), 29 deletions(-)
Approvals:
Internal Jenkins: Verified
Tim Armstrong: Looks good to me, approved
--
To view, visit http://gerrit.cloudera.org:8080/4290
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: Ia9dfcca718b2456488d0fb4407d3817c796bfad0
Gerrit-PatchSet: 6
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Internal Jenkins
Gerrit-Reviewer: Jim Apple <jb...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
[Impala-ASF-CR] IMPALA-4058: benchmark byteswap on misaligned memory
Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Tim Armstrong has posted comments on this change.
Change subject: IMPALA-4058: benchmark byteswap on misaligned memory
......................................................................
Patch Set 4:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/4290/4/be/src/benchmarks/bswap-benchmark.cc
File be/src/benchmarks/bswap-benchmark.cc:
Line 20: #include <iostream>
> nit: https://google.github.io/styleguide/cppguide.html#Names_and_Order_of_I
Done
--
To view, visit http://gerrit.cloudera.org:8080/4290
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia9dfcca718b2456488d0fb4407d3817c796bfad0
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Internal Jenkins
Gerrit-Reviewer: Jim Apple <jb...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4058: benchmark byteswap on misaligned memory
Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Tim Armstrong has posted comments on this change.
Change subject: IMPALA-4058: benchmark byteswap on misaligned memory
......................................................................
Patch Set 3:
Will wait until https://gerrit.cloudera.org/#/c/4205/6 goes in.
--
To view, visit http://gerrit.cloudera.org:8080/4290
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia9dfcca718b2456488d0fb4407d3817c796bfad0
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Jim Apple <jb...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4058: benchmark byteswap on misaligned memory
Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Tim Armstrong has uploaded a new patch set (#3).
Change subject: IMPALA-4058: benchmark byteswap on misaligned memory
......................................................................
IMPALA-4058: benchmark byteswap on misaligned memory
Extend the benchmark to measure performance when memory is misaligned.
This is a common case when decoding parquet.
Change-Id: Ia9dfcca718b2456488d0fb4407d3817c796bfad0
---
M be/src/benchmarks/bswap-benchmark.cc
1 file changed, 73 insertions(+), 26 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/90/4290/3
--
To view, visit http://gerrit.cloudera.org:8080/4290
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ia9dfcca718b2456488d0fb4407d3817c796bfad0
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Jim Apple <jb...@cloudera.com>
[Impala-ASF-CR] IMPALA-4058: benchmark byteswap on misaligned memory
Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Tim Armstrong has posted comments on this change.
Change subject: IMPALA-4058: benchmark byteswap on misaligned memory
......................................................................
Patch Set 1:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/4290/1/be/src/benchmarks/bswap-benchmark.cc
File be/src/benchmarks/bswap-benchmark.cc:
PS1, Line 120: perfectly aligned of SIMD
> I'm not sure these are guaranteed to be aligned. I assume the array that ve
Good point. Used posix_memalign to 64-bytes (so that it's aligned to cache lines too.
PS1, Line 122: vector
> const, here and in the range-based for loop
Done
--
To view, visit http://gerrit.cloudera.org:8080/4290
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia9dfcca718b2456488d0fb4407d3817c796bfad0
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Jim Apple <jb...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4058: benchmark byteswap on misaligned memory
Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Hello Jim Apple, Internal Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/4290
to look at the new patch set (#5).
Change subject: IMPALA-4058: benchmark byteswap on misaligned memory
......................................................................
IMPALA-4058: benchmark byteswap on misaligned memory
Extend the benchmark to measure performance when memory is misaligned.
This is a common case when decoding parquet.
Change-Id: Ia9dfcca718b2456488d0fb4407d3817c796bfad0
---
M be/src/benchmarks/bswap-benchmark.cc
M be/src/common/names.h
2 files changed, 77 insertions(+), 29 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/90/4290/5
--
To view, visit http://gerrit.cloudera.org:8080/4290
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ia9dfcca718b2456488d0fb4407d3817c796bfad0
Gerrit-PatchSet: 5
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Internal Jenkins
Gerrit-Reviewer: Jim Apple <jb...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
[Impala-ASF-CR] IMPALA-4058: benchmark byteswap on misaligned memory
Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Tim Armstrong has uploaded a new patch set (#2).
Change subject: IMPALA-4058: benchmark byteswap on misaligned memory
......................................................................
IMPALA-4058: benchmark byteswap on misaligned memory
Extend the benchmark to measure performance when memory is misaligned.
This is a common case when decoding parquet.
Change-Id: Ia9dfcca718b2456488d0fb4407d3817c796bfad0
---
M be/src/benchmarks/bswap-benchmark.cc
1 file changed, 73 insertions(+), 26 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/90/4290/2
--
To view, visit http://gerrit.cloudera.org:8080/4290
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ia9dfcca718b2456488d0fb4407d3817c796bfad0
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Jim Apple <jb...@cloudera.com>
[Impala-ASF-CR] IMPALA-4058: benchmark byteswap on misaligned memory
Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Tim Armstrong has posted comments on this change.
Change subject: IMPALA-4058: benchmark byteswap on misaligned memory
......................................................................
Patch Set 5: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/4290
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia9dfcca718b2456488d0fb4407d3817c796bfad0
Gerrit-PatchSet: 5
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Internal Jenkins
Gerrit-Reviewer: Jim Apple <jb...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4058: benchmark byteswap on misaligned memory
Posted by "Jim Apple (Code Review)" <ge...@cloudera.org>.
Jim Apple has posted comments on this change.
Change subject: IMPALA-4058: benchmark byteswap on misaligned memory
......................................................................
Patch Set 1:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/4290/1/be/src/benchmarks/bswap-benchmark.cc
File be/src/benchmarks/bswap-benchmark.cc:
PS1, Line 120: perfectly aligned of SIMD
I'm not sure these are guaranteed to be aligned. I assume the array that vector allocates on the heap is only aligned as much as the memory manager and the underlying data type require.
You can force it by using posix_memalign, or just picking the starting byte for data.inbuffer based on the alignment of the first byte in the vector.
PS1, Line 122: vector
const, here and in the range-based for loop
--
To view, visit http://gerrit.cloudera.org:8080/4290
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia9dfcca718b2456488d0fb4407d3817c796bfad0
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Jim Apple <jb...@cloudera.com>
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4058: benchmark byteswap on misaligned memory
Posted by "Internal Jenkins (Code Review)" <ge...@cloudera.org>.
Internal Jenkins has posted comments on this change.
Change subject: IMPALA-4058: benchmark byteswap on misaligned memory
......................................................................
Patch Set 4: Verified-1
Build failed: http://sandbox.jenkins.cloudera.com/job/impala-external-gerrit-verify-merge-ASF/164/
--
To view, visit http://gerrit.cloudera.org:8080/4290
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia9dfcca718b2456488d0fb4407d3817c796bfad0
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Internal Jenkins
Gerrit-Reviewer: Jim Apple <jb...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: No