Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/24132 )
Change subject: IMPALA-14851: Codegen GroupingAggregator::CopyGroupingValues ...................................................................... Patch Set 10: Code-Review+1 (3 comments) http://gerrit.cloudera.org:8080/#/c/24132/10//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/24132/10//COMMIT_MSG@18 PS10, Line 18: AGGREGATION_NODE Avg Time changed from 743.281ms to 659.414ms. Can you add the difference in the profile for some codegen metrics for affected fragments? For example NumInstructions and ModuleBitcodeSize http://gerrit.cloudera.org:8080/#/c/24132/10/be/src/codegen/llvm-codegen.h File be/src/codegen/llvm-codegen.h: http://gerrit.cloudera.org:8080/#/c/24132/10/be/src/codegen/llvm-codegen.h@634 PS10, Line 634: /// Codegens IR to call the function corresponding to 'ir_type' with argument 'args' : /// and returns the value. : llvm::Value* CodegenCallFunction(LlvmBuilder* builder, IRFunction::Type ir_type, : llvm::ArrayRef<llvm::Value*> args, const char* name); : : /// Codegens IR to call the function without return value (i.e. returning void) with : /// argument 'args'. Returns the generated llvm::CallInst*. The returned value doesn't : /// have a result value in LLVM IR. The difference between these functions seem to be that the new function has not name - can you make the effect of this clearer in the comments? Also, the two could be unified by setting default nullptr for name. http://gerrit.cloudera.org:8080/#/c/24132/10/be/src/exec/grouping-aggregator.cc File be/src/exec/grouping-aggregator.cc: http://gerrit.cloudera.org:8080/#/c/24132/10/be/src/exec/grouping-aggregator.cc@1284 PS10, Line 1284: // call void @_ZN6impala18GroupingAggregator30CopyGroupingValuesFixedLenSlot... just curious, when do we get this output? I assumed that we'll never see IR_ALWAYS_INLINE functions in IR output -- To view, visit http://gerrit.cloudera.org:8080/24132 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia2c54a5745ba05a926795318d3b074fde6d0c00a Gerrit-Change-Number: 24132 Gerrit-PatchSet: 10 Gerrit-Owner: Balazs Hevele <[email protected]> Gerrit-Reviewer: Balazs Hevele <[email protected]> Gerrit-Reviewer: Csaba Ringhofer <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Yida Wu <[email protected]> Gerrit-Comment-Date: Tue, 09 Jun 2026 11:53:59 +0000 Gerrit-HasComments: Yes
