Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/10421 )
Change subject: IMPALA-5168: Codegen HASH_PARTITIONED KrpcDataStreamSender::Send() ...................................................................... Patch Set 2: (1 comment) Code looks good, just want to be sure we get ahead of any codegen time issues :) http://gerrit.cloudera.org:8080/#/c/10421/2/be/src/runtime/krpc-data-stream-sender.cc File be/src/runtime/krpc-data-stream-sender.cc: http://gerrit.cloudera.org:8080/#/c/10421/2/be/src/runtime/krpc-data-stream-sender.cc@716 PS2, Line 716: for (int i = 0; i < partition_exprs_.size(); ++i) { Have you done any experiments for wide rows to determine the codegen time? Unrolling a loop N times is the classic pattern that we've seen cause codegen time to blow up before. It would be good to get ahead of it in this instance and either do something to mitigate it (like http://gerrit.cloudera.org:8080/8211), or disable it above some number of expressions to avoid the regression -- To view, visit http://gerrit.cloudera.org:8080/10421 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1c44cc9312c062cc7a5a4ac9156ceaa31fb887ff Gerrit-Change-Number: 10421 Gerrit-PatchSet: 2 Gerrit-Owner: Michael Ho <[email protected]> Gerrit-Reviewer: Bikramjeet Vig <[email protected]> Gerrit-Reviewer: Michael Ho <[email protected]> Gerrit-Reviewer: Tim Armstrong <[email protected]> Gerrit-Comment-Date: Mon, 21 May 2018 16:14:02 +0000 Gerrit-HasComments: Yes
