You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Mostafa Mokhtar (Code Review)" <ge...@cloudera.org> on 2018/02/20 21:46:07 UTC
[Impala-ASF-CR] IMPALA-6461 : Micro-optimizations to DataStreamSender::Send
Mostafa Mokhtar has uploaded this change for review. ( http://gerrit.cloudera.org:8080/9221
Change subject: IMPALA-6461 : Micro-optimizations to DataStreamSender::Send
......................................................................
IMPALA-6461 : Micro-optimizations to DataStreamSender::Send
While analyzing performance of partition exchange operator,
I noticed that there is dependency and a function call per row in the hot path.
Optimizations in this change are:
1) Remove the data dependency between computing the hash and the channel
2) Inline DataStreamSender::Channel::AddRow
3) Save partition_exprs_.size() to save a couple of instructions
This translates to improving CPI for DataStreamSender::Send by 10%
Change-Id: I642a9dad531a29d4838a3537ab0e04320a69960d
---
M be/src/runtime/data-stream-sender.cc
M be/src/runtime/krpc-data-stream-sender.cc
M be/src/runtime/row-batch.h
3 files changed, 109 insertions(+), 48 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/21/9221/6
--
To view, visit http://gerrit.cloudera.org:8080/9221
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I642a9dad531a29d4838a3537ab0e04320a69960d
Gerrit-Change-Number: 9221
Gerrit-PatchSet: 6
Gerrit-Owner: Mostafa Mokhtar <mm...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Mostafa Mokhtar <mm...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
[Impala-ASF-CR] IMPALA-6461 : Micro-optimizations to DataStreamSender::Send
Posted by "Mostafa Mokhtar (Code Review)" <ge...@cloudera.org>.
Mostafa Mokhtar has uploaded a new patch set (#7). ( http://gerrit.cloudera.org:8080/9221 )
Change subject: IMPALA-6461 : Micro-optimizations to DataStreamSender::Send
......................................................................
IMPALA-6461 : Micro-optimizations to DataStreamSender::Send
While analyzing performance of partition exchange operator,
I noticed that there is dependency and a function call per row in the hot path.
Optimizations in this change are:
1) Remove the data dependency between computing the hash and the channel
2) Inline DataStreamSender::Channel::AddRow
3) Save partition_exprs_.size() to save a couple of instructions
This translates to improving CPI for DataStreamSender::Send by 10%
Change-Id: I642a9dad531a29d4838a3537ab0e04320a69960d
---
M be/src/runtime/data-stream-sender.cc
M be/src/runtime/krpc-data-stream-sender.cc
M be/src/runtime/row-batch.h
3 files changed, 110 insertions(+), 48 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/21/9221/7
--
To view, visit http://gerrit.cloudera.org:8080/9221
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I642a9dad531a29d4838a3537ab0e04320a69960d
Gerrit-Change-Number: 9221
Gerrit-PatchSet: 7
Gerrit-Owner: Mostafa Mokhtar <mm...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Mostafa Mokhtar <mm...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
[Impala-ASF-CR] IMPALA-6461 : Micro-optimizations to DataStreamSender::Send
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/9221 )
Change subject: IMPALA-6461 : Micro-optimizations to DataStreamSender::Send
......................................................................
Patch Set 8:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/1974/
--
To view, visit http://gerrit.cloudera.org:8080/9221
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I642a9dad531a29d4838a3537ab0e04320a69960d
Gerrit-Change-Number: 9221
Gerrit-PatchSet: 8
Gerrit-Owner: Mostafa Mokhtar <mm...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Mostafa Mokhtar <mm...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Wed, 21 Feb 2018 18:32:21 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6461 : Micro-optimizations to DataStreamSender::Send
Posted by "Mostafa Mokhtar (Code Review)" <ge...@cloudera.org>.
Mostafa Mokhtar has posted comments on this change. ( http://gerrit.cloudera.org:8080/9221 )
Change subject: IMPALA-6461 : Micro-optimizations to DataStreamSender::Send
......................................................................
Patch Set 8: Code-Review+2
Carry +2
--
To view, visit http://gerrit.cloudera.org:8080/9221
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I642a9dad531a29d4838a3537ab0e04320a69960d
Gerrit-Change-Number: 9221
Gerrit-PatchSet: 8
Gerrit-Owner: Mostafa Mokhtar <mm...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Mostafa Mokhtar <mm...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Wed, 21 Feb 2018 18:29:22 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6461 : Micro-optimizations to DataStreamSender::Send
Posted by "Mostafa Mokhtar (Code Review)" <ge...@cloudera.org>.
Mostafa Mokhtar has uploaded a new patch set (#8). ( http://gerrit.cloudera.org:8080/9221 )
Change subject: IMPALA-6461 : Micro-optimizations to DataStreamSender::Send
......................................................................
IMPALA-6461 : Micro-optimizations to DataStreamSender::Send
While analyzing performance of partition exchange operator,
I noticed that there is dependency and a function call per row in the hot path.
Optimizations in this change are:
1) Remove the data dependency between computing the hash and the channel
2) Inline DataStreamSender::Channel::AddRow
3) Save partition_exprs_.size() to save a couple of instructions
This translates to improving CPI for DataStreamSender::Send by 10%
Change-Id: I642a9dad531a29d4838a3537ab0e04320a69960d
---
M be/src/runtime/data-stream-sender.cc
M be/src/runtime/krpc-data-stream-sender.cc
M be/src/runtime/row-batch.h
3 files changed, 110 insertions(+), 48 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/21/9221/8
--
To view, visit http://gerrit.cloudera.org:8080/9221
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I642a9dad531a29d4838a3537ab0e04320a69960d
Gerrit-Change-Number: 9221
Gerrit-PatchSet: 8
Gerrit-Owner: Mostafa Mokhtar <mm...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Mostafa Mokhtar <mm...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
[Impala-ASF-CR] IMPALA-6461 : Micro-optimizations to DataStreamSender::Send
Posted by "Mostafa Mokhtar (Code Review)" <ge...@cloudera.org>.
Mostafa Mokhtar has posted comments on this change. ( http://gerrit.cloudera.org:8080/9221 )
Change subject: IMPALA-6461 : Micro-optimizations to DataStreamSender::Send
......................................................................
Patch Set 7:
(4 comments)
http://gerrit.cloudera.org:8080/#/c/9221/7/be/src/runtime/data-stream-sender.cc
File be/src/runtime/data-stream-sender.cc:
http://gerrit.cloudera.org:8080/#/c/9221/7/be/src/runtime/data-stream-sender.cc@451
PS7, Line 451: (num_rows - batch_start)
> nit: unnecessary parenthesis
Done
http://gerrit.cloudera.org:8080/#/c/9221/7/be/src/runtime/data-stream-sender.cc@485
PS7, Line 485: (num_rows - batch_start)
> Same here
Done
http://gerrit.cloudera.org:8080/#/c/9221/7/be/src/runtime/krpc-data-stream-sender.cc
File be/src/runtime/krpc-data-stream-sender.cc:
http://gerrit.cloudera.org:8080/#/c/9221/7/be/src/runtime/krpc-data-stream-sender.cc@668
PS7, Line 668: (num_rows - batch_start)
> Same here.
Done
http://gerrit.cloudera.org:8080/#/c/9221/7/be/src/runtime/krpc-data-stream-sender.cc@702
PS7, Line 702: (num_rows - batch_start)
> Same here.
Done
--
To view, visit http://gerrit.cloudera.org:8080/9221
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I642a9dad531a29d4838a3537ab0e04320a69960d
Gerrit-Change-Number: 9221
Gerrit-PatchSet: 7
Gerrit-Owner: Mostafa Mokhtar <mm...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Mostafa Mokhtar <mm...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Wed, 21 Feb 2018 18:29:56 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6461 : Micro-optimizations to DataStreamSender::Send
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/9221 )
Change subject: IMPALA-6461 : Micro-optimizations to DataStreamSender::Send
......................................................................
IMPALA-6461 : Micro-optimizations to DataStreamSender::Send
While analyzing performance of partition exchange operator,
I noticed that there is dependency and a function call per row in the hot path.
Optimizations in this change are:
1) Remove the data dependency between computing the hash and the channel
2) Inline DataStreamSender::Channel::AddRow
3) Save partition_exprs_.size() to save a couple of instructions
This translates to improving CPI for DataStreamSender::Send by 10%
Change-Id: I642a9dad531a29d4838a3537ab0e04320a69960d
Reviewed-on: http://gerrit.cloudera.org:8080/9221
Reviewed-by: Mostafa Mokhtar <mm...@cloudera.com>
Tested-by: Impala Public Jenkins
---
M be/src/runtime/data-stream-sender.cc
M be/src/runtime/krpc-data-stream-sender.cc
M be/src/runtime/row-batch.h
3 files changed, 110 insertions(+), 48 deletions(-)
Approvals:
Mostafa Mokhtar: Looks good to me, approved
Impala Public Jenkins: Verified
--
To view, visit http://gerrit.cloudera.org:8080/9221
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I642a9dad531a29d4838a3537ab0e04320a69960d
Gerrit-Change-Number: 9221
Gerrit-PatchSet: 9
Gerrit-Owner: Mostafa Mokhtar <mm...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Mostafa Mokhtar <mm...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
[Impala-ASF-CR] IMPALA-6461 : Micro-optimizations to DataStreamSender::Send
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/9221 )
Change subject: IMPALA-6461 : Micro-optimizations to DataStreamSender::Send
......................................................................
Patch Set 8: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/9221
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I642a9dad531a29d4838a3537ab0e04320a69960d
Gerrit-Change-Number: 9221
Gerrit-PatchSet: 8
Gerrit-Owner: Mostafa Mokhtar <mm...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Mostafa Mokhtar <mm...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Wed, 21 Feb 2018 22:11:21 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6461 : Micro-optimizations to DataStreamSender::Send
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/9221 )
Change subject: IMPALA-6461 : Micro-optimizations to DataStreamSender::Send
......................................................................
Patch Set 8:
Build started: https://jenkins.impala.io/job/gerrit-docs-submit/200/
--
To view, visit http://gerrit.cloudera.org:8080/9221
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I642a9dad531a29d4838a3537ab0e04320a69960d
Gerrit-Change-Number: 9221
Gerrit-PatchSet: 8
Gerrit-Owner: Mostafa Mokhtar <mm...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Mostafa Mokhtar <mm...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Wed, 21 Feb 2018 18:30:57 +0000
Gerrit-HasComments: No