Tim Armstrong has posted comments on this change. Change subject: IMPALA-4231: fix codegen time regression ......................................................................
Patch Set 1: (4 comments) http://gerrit.cloudera.org:8080/#/c/4623/1//COMMIT_MSG Commit Message: PS1, Line 26: y, which is fixed by this : patch. > Does it recover all the performance ? Locally it looked like it got back all of the codegen time plus some more. I've had trouble getting a full cluster run completed, but it looks like it got TPC-H Q2 back to the 4.5 second range. http://gerrit.cloudera.org:8080/#/c/4623/1/be/src/exec/partitioned-hash-join-builder-ir.cc File be/src/exec/partitioned-hash-join-builder-ir.cc: PS1, Line 32: inline bool PhjBuilder::AppendRow( > I thought the destructor is inlined so it only calls FreeMessage() which is The codegen time was higher. Otherwise the runtime didn't seem to be noticeably higher. Looking at the IR the Statuses added 5-10 additional basic blocks to the Process*Batch functions that were already pretty huge. Basically every local Status variable or return of a Status variable was adding calls to the Copy/FreeMsg function that weren't optimised out. I posted the IR diff as patchsets 1 and 2 here. http://gerrit.cloudera.org:8080/#/c/4623/1/be/src/runtime/raw-value.inline.h File be/src/runtime/raw-value.inline.h: PS1, Line 215: > this may need to be inlined again eventually for loop unrolling to work. I agree, I think we should move it to the -ir.cc file once we're able to propagate constants into it. It doesn't make sense to burn cycles recompiling the whole interpreted function all the time. It will be cheaper to optimise the specialised version without the big switch. http://gerrit.cloudera.org:8080/#/c/4623/1/be/src/util/bloom-filter.h File be/src/util/bloom-filter.h: PS1, Line 176: inline void IR_ALWAYS_INLINE > void ALWAYS_INLINE 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: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Tim Armstrong <tarmstr...@cloudera.com> Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Michael Ho <k...@cloudera.com> Gerrit-Reviewer: Tim Armstrong <tarmstr...@cloudera.com> Gerrit-HasComments: Yes
