Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/19181 )
Change subject: IMPALA-11470: Add Cache For Codegen Functions ...................................................................... Patch Set 5: (12 comments) http://gerrit.cloudera.org:8080/#/c/19181/3/be/src/codegen/llvm-codegen-cache.h File be/src/codegen/llvm-codegen-cache.h: http://gerrit.cloudera.org:8080/#/c/19181/3/be/src/codegen/llvm-codegen-cache.h@95 PS3, Line 95: /// It could be in future to construct from a list of string keys. > Done Done http://gerrit.cloudera.org:8080/#/c/19181/3/be/src/codegen/llvm-codegen.h File be/src/codegen/llvm-codegen.h: http://gerrit.cloudera.org:8080/#/c/19181/3/be/src/codegen/llvm-codegen.h@191 PS3, Line 191: void EnableOptimizations(bool enable); > Done Done http://gerrit.cloudera.org:8080/#/c/19181/3/be/src/codegen/llvm-codegen.h@807 PS3, Line 807: > Done Done http://gerrit.cloudera.org:8080/#/c/19181/3/be/src/codegen/llvm-codegen.h@933 PS3, Line 933: > The CodeGenCache instance is a singleton in the daemon, while the construct Done http://gerrit.cloudera.org:8080/#/c/19181/3/be/src/codegen/llvm-codegen.cc File be/src/codegen/llvm-codegen.cc: http://gerrit.cloudera.org:8080/#/c/19181/3/be/src/codegen/llvm-codegen.cc@97 PS3, Line 97: > This doesn't seem to be used. Done http://gerrit.cloudera.org:8080/#/c/19181/3/be/src/exprs/scalar-expr.h File be/src/exprs/scalar-expr.h: http://gerrit.cloudera.org:8080/#/c/19181/3/be/src/exprs/scalar-expr.h@254 PS3, Line 254: bool AllowCodegenCache() const { return allow_codegen_cache_; } > Done Done http://gerrit.cloudera.org:8080/#/c/19181/3/be/src/runtime/exec-env.h File be/src/runtime/exec-env.h: http://gerrit.cloudera.org:8080/#/c/19181/3/be/src/runtime/exec-env.h@156 PS3, Line 156: bool codegen_cache_enabled() const { return codegen_cache_ != nullptr; } > Done Done http://gerrit.cloudera.org:8080/#/c/19181/3/be/src/runtime/fragment-state.h File be/src/runtime/fragment-state.h: http://gerrit.cloudera.org:8080/#/c/19181/3/be/src/runtime/fragment-state.h@75 PS3, Line 75: bool codegen_cache_enabled() const { > Done Done http://gerrit.cloudera.org:8080/#/c/19181/3/be/src/runtime/fragment-state.h@92 PS3, Line 92: bool AllowCodegenCache() const { return allow_codegen_cache_; } > Done Done http://gerrit.cloudera.org:8080/#/c/19181/3/be/src/runtime/fragment-state.h@231 PS3, Line 231: /// Indicates whether codegen cache is allowed. If the fragment contains udf > Done Done http://gerrit.cloudera.org:8080/#/c/19181/3/be/src/runtime/krpc-data-stream-sender.cc File be/src/runtime/krpc-data-stream-sender.cc: http://gerrit.cloudera.org:8080/#/c/19181/3/be/src/runtime/krpc-data-stream-sender.cc@93 PS3, Line 93: state->CheckAndAddCodegenDisabledMessage(codegen_status_msgs_); > Thanks for pointing out this. I moved the seed out of the codegen function, I think I understand what's going on here. We now pass exchange_hash_seed_ to the codegen'd function at runtime via HashAndAddRows. That makes sense. http://gerrit.cloudera.org:8080/#/c/19181/3/be/src/runtime/query-state.h File be/src/runtime/query-state.h: http://gerrit.cloudera.org:8080/#/c/19181/3/be/src/runtime/query-state.h@154 PS3, Line 154: bool codegen_cache_enabled() const; > Done Done -- To view, visit http://gerrit.cloudera.org:8080/19181 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If42c78a7f51fd582e5fe331fead494dadf544eb1 Gerrit-Change-Number: 19181 Gerrit-PatchSet: 5 Gerrit-Owner: Yida Wu <[email protected]> Gerrit-Reviewer: Abhishek Rawat <[email protected]> Gerrit-Reviewer: Daniel Becker <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Joe McDonnell <[email protected]> Gerrit-Reviewer: Michael Smith <[email protected]> Gerrit-Reviewer: Yida Wu <[email protected]> Gerrit-Comment-Date: Thu, 10 Nov 2022 00:56:02 +0000 Gerrit-HasComments: Yes
