[ 
https://issues.apache.org/jira/browse/IMPALA-12390?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17765781#comment-17765781
 ] 

ASF subversion and git services commented on IMPALA-12390:
----------------------------------------------------------

Commit f563cce6b8eed4519772af06d89edc6e374839d3 in impala's branch 
refs/heads/branch-4.3.0 from Joe McDonnell
[ https://gitbox.apache.org/repos/asf?p=impala.git;h=f563cce6b ]

IMPALA-12390 (part 1): Enable some clang-tidy performance related checks

This enables several Clang Tidy performance checks and fixes
the flagged code locations. The specific checks enabled are:
1. performance-faster-string-find
  "warning: 'find' called with a string literal consisting of a
   single character; consider using the more effective overload
   accepting a character"
  Fix: Use char literals rather than string literals
2. performance-for-range-copy
  "warning: loop variable is copied but only used as const reference;
   consider making it a const reference"
  Fix: Use const & for some locations of auto
3. performance-implicit-cast-in-loop
  "warning: the type of the loop variable '$VAR' is different from
   the one returned by the iterator and generates an implicit cast;
   you can either change the type to the correct one ('$TYPE' but
   'const auto&' is always an option) or remove the reference to make
   it explicit that you are creating a new value"
  Fix: Use the right type or const auto&
4. performance-inefficient-vector-operation
  "warning: 'push_back' is called inside a loop; consider pre-allocating
   the vector capacity before the loop"
  Fix: Call reserve() on the vector before the loop
5. performance-type-promotion-in-math-fn - not encountered in our code

This disables a few performance checks temporarily to keep the
change a reasonable size.

Testing:
 - Ran bin/run_clang_tidy.sh with the new checks
 - Ran GVO

Change-Id: I3d5dfe04ffb4ec6f156e268c31a356651410ce91
Reviewed-on: http://gerrit.cloudera.org:8080/20387
Tested-by: Impala Public Jenkins <[email protected]>
Reviewed-by: Michael Smith <[email protected]>


> Enable performance related clang-tidy checks
> --------------------------------------------
>
>                 Key: IMPALA-12390
>                 URL: https://issues.apache.org/jira/browse/IMPALA-12390
>             Project: IMPALA
>          Issue Type: Improvement
>          Components: Backend
>    Affects Versions: Impala 4.3.0
>            Reporter: Joe McDonnell
>            Assignee: Joe McDonnell
>            Priority: Major
>             Fix For: Impala 4.3.0
>
>
> clang-tidy has several performance-related checks that seem like they would 
> be useful to enforce. Here are some examples:
> {noformat}
> /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);
>     ^{noformat}
> 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.



--
This message was sent by Atlassian Jira
(v8.20.10#820010)

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to