Bikramjeet Vig has posted comments on this change. ( http://gerrit.cloudera.org:8080/15408 )
Change subject: IMPALA-4080 [part 7]: Codegen once per fragment ...................................................................... Patch Set 10: (6 comments) http://gerrit.cloudera.org:8080/#/c/15408/7//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/15408/7//COMMIT_MSG@18 PS7, Line 18: parent fragment does the actual codegen work and the rest of the > Maybe include an example of how the codegen in the profile looks after the Done http://gerrit.cloudera.org:8080/#/c/15408/8//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/15408/8//COMMIT_MSG@26 PS8, Line 26: An example of how the codegen in the profile looks: > This can be updated right? Done http://gerrit.cloudera.org:8080/#/c/15408/8/be/src/runtime/fragment-state.h File be/src/runtime/fragment-state.h: http://gerrit.cloudera.org:8080/#/c/15408/8/be/src/runtime/fragment-state.h@42 PS8, Line 42: QueryState* state, std::unordered_map<TFragmentIdx, FragmentState*>& fragment_map); > Missed this TODO Done http://gerrit.cloudera.org:8080/#/c/15408/8/be/src/runtime/fragment-state.h@168 PS8, Line 168: Status codegen_status_; > Same here Done http://gerrit.cloudera.org:8080/#/c/15408/8/be/src/runtime/fragment-state.cc File be/src/runtime/fragment-state.cc: http://gerrit.cloudera.org:8080/#/c/15408/8/be/src/runtime/fragment-state.cc@84 PS8, Line 84: codegen_status_.AddDetail(error_ctx); > It would be good to do this outside of the critical section so that we don' As we discussed offline, keeping this unchanged to avoid fragment instance threads racing to set the error status in queryState. Also documented the lock ordering over codegen_lock_ http://gerrit.cloudera.org:8080/#/c/15408/8/be/src/runtime/query-state.cc File be/src/runtime/query-state.cc: http://gerrit.cloudera.org:8080/#/c/15408/8/be/src/runtime/query-state.cc@548 PS8, Line 548: if (!HasErrorStatus()) { > nit: extra braces for lock scope not needed Done -- To view, visit http://gerrit.cloudera.org:8080/15408 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3aef8bc621f96caafe9a1c378617a2987e4ad452 Gerrit-Change-Number: 15408 Gerrit-PatchSet: 10 Gerrit-Owner: Bikramjeet Vig <[email protected]> Gerrit-Reviewer: Bikramjeet Vig <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Tim Armstrong <[email protected]> Gerrit-Comment-Date: Tue, 31 Mar 2020 20:16:44 +0000 Gerrit-HasComments: Yes
