[IMPALA-12390] Enable performance related clang-tidy checks

• Type: Improvement
• Status: In Progress
• Priority: Major
• Resolution: Unresolved
• Affects Version/s: Impala 4.3.0
• Fix Version/s: Impala 4.5.0
• Component/s: Backend
• Epic Color: ghx-label-12
• Assignee: Joe McDonnell
• Reporter: Joe McDonnell
• Created: 22/Aug/23 00:17
• Updated: 1 week ago 08:38

Description

clang-tidy has several performance-related checks that seem like they would be useful to enforce. Here are some examples:

/home/joemcdonnell/upstream/Impala/be/src/runtime/types.h:313:25: warning: loop variable is copied but only used as const reference; consider making it a const reference [performance-for-range-copy]
        for (ColumnType child_type : col_type.children) {
             ~~~~~~~~~~ ^
             const &

/home/joemcdonnell/upstream/Impala/be/src/catalog/catalog-util.cc:168:34: warning: 'find' called with a string literal consisting of a single character; consider using the more effective overload accepting a character [performance-faster-string-find]
      int pos = object_name.find(".");
                                 ^~~~
                                 '.'

/home/joemcdonnell/upstream/Impala/be/src/util/decimal-util.h:55:53: warning: the parameter 'b' is copied for each invocation but only used as a const reference; consider making it a const reference [performance-unnecessary-value-param]
  static int256_t SafeMultiply(int256_t a, int256_t b, bool may_overflow) {
                                           ~~~~~~~~ ^
                                           const &

/home/joemcdonnell/upstream/Impala/be/src/codegen/llvm-codegen.cc:847:5: warning: 'push_back' is called inside a loop; consider pre-allocating the vector capacity before the loop [performance-inefficient-vector-operation]
    arguments.push_back(args_[i].type);
    ^

In all, they seem to flag things that developers wouldn't ordinarily notice, and it doesn't seem to have too many false positives. We should look into enabling these.

Issue Links

• is related to: IMPALA-12393 DictEncoder uses inconsistent hash function for TimestampValue