Tim Armstrong has posted comments on this change. Change subject: IMPALA-4231: fix codegen time regression ......................................................................
Patch Set 3: (6 comments) http://gerrit.cloudera.org:8080/#/c/4623/3/be/src/exec/partitioned-hash-join-builder-ir.cc File be/src/exec/partitioned-hash-join-builder-ir.cc: PS3, Line 35: if (!status->ok()) > This path seems to be hot enough to warrant if (UNLIKELY(!status->ok()). Done. Line 51: if (!AppendRow(null_aware_partition_->build_rows(), build_row, &status)) { > UNLIKELY ? Done Line 72: if (!AppendRow(partition->build_rows(), build_row, &status)) { > UNLIKELY ? Done http://gerrit.cloudera.org:8080/#/c/4623/3/be/src/exec/partitioned-hash-join-node-ir.cc File be/src/exec/partitioned-hash-join-node-ir.cc: Line 286: if (!AppendProbeRow(null_probe_rows_.get(), current_probe_row_, status)) { > May warrant an UNLIKELY here. Done Line 312: if (!AppendProbeRow(probe_rows, current_probe_row_, status)) { > May warrant an UNLIKELY here. Done http://gerrit.cloudera.org:8080/#/c/4623/3/be/src/runtime/raw-value.inline.h File be/src/runtime/raw-value.inline.h: PS3, Line 215: RawValue::PrintValue > It appears that this really shouldn't be inlined either. Done -- To view, visit http://gerrit.cloudera.org:8080/4623 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Idf0fdedabd488550b6db90167a30c582949d608d Gerrit-PatchSet: 3 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Tim Armstrong <tarmstr...@cloudera.com> Gerrit-Reviewer: Marcel Kornacker <mar...@cloudera.com> Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Michael Ho <k...@cloudera.com> Gerrit-Reviewer: Tim Armstrong <tarmstr...@cloudera.com> Gerrit-HasComments: Yes
