Michael Ho has posted comments on this change. Change subject: IMPALA-3066: Lazy materialization of LLVM module bitcode. ......................................................................
Patch Set 1: (4 comments) http://gerrit.cloudera.org:8080/#/c/3220/1/be/src/codegen/llvm-codegen.cc File be/src/codegen/llvm-codegen.cc: Line 76: DEFINE_bool(disable_lazy_materialization, false, > Maybe we should chat a bit offline. If there are some gaps in test coverage Sure, I think we can remove this flag. Just thought it would be safe to add a way to disable them if necessary but yes, we shouldn't need it if we are confident about our test coverage. Line 97: static StringRef llvm_module_ir; > It's still some additional effort to reason about when they are initialised Will it be better if they are class static ? One way to look at them is that they are in the same category as cpu_attrs_ or cpu_name_ which are host specific and initialized once at start up time. Line 797: nullptr > There are some obscure type differences but I think NULL should work ok (it It wasn't immediately clear to me nullptr == NULL. That's why I am a bit hesitant in just using NULL but I can give it a try. http://gerrit.cloudera.org:8080/#/c/3220/1/be/src/runtime/buffered-tuple-stream.inline.h File be/src/runtime/buffered-tuple-stream.inline.h: Line 58 > I'm ok with this, maybe just mention in commit message? Will do. -- To view, visit http://gerrit.cloudera.org:8080/3220 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I6ed7862fc5e86005ecea83fa2ceb489e737d66b2 Gerrit-PatchSet: 1 Gerrit-Project: Impala Gerrit-Branch: cdh5-trunk Gerrit-Owner: Michael Ho <[email protected]> Gerrit-Reviewer: Michael Ho <[email protected]> Gerrit-Reviewer: Tim Armstrong <[email protected]> Gerrit-HasComments: Yes
