Bug 192666 – clang-tidy: Fix unnecessary parameter copies in ParallelHelperPool.cpp

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Template Framework

[David Kilzer (:ddkilzer)]

Running `clang-tidy -checks='-*,performance-*,-performance-noexcept-*' ...` on ParallelHelperPool.cpp found these unnecessary copies:

Source/WTF/wtf/ParallelHelperPool.cpp:36:14: warning: parameter 'pool' is passed by value and only copied once; consider moving it to avoid unnecessary copies [performance-unnecessary-value-param]
    : m_pool(pool)
             ^
             std::move( )
Source/WTF/wtf/ParallelHelperPool.cpp:61:14: warning: parameter 'task' is passed by value and only copied once; consider moving it to avoid unnecessary copies [performance-unnecessary-value-param]
    m_task = task;
             ^
             std::move( )
Source/WTF/wtf/ParallelHelperPool.cpp:86:13: warning: parameter 'task' is passed by value and only copied once; consider moving it to avoid unnecessary copies [performance-unnecessary-value-param]
    setTask(task);
            ^
            std::move( )
Source/WTF/wtf/ParallelHelperPool.cpp:107:64: warning: the parameter 'task' is copied for each invocation but only used as a const reference; consider making it a const reference [performance-unnecessary-value-param]
void ParallelHelperClient::runTask(RefPtr<SharedTask<void ()>> task)
                                                               ^
                                   const                      &

[Radar WebKit Bug Importer]

<rdar://problem/46697952>

[David Kilzer (:ddkilzer)]

Created attachment 357233 [details]
Patch v1

[EWS Watchlist]

Attachment 357233 [details] did not pass style-queue:


ERROR: Source/WTF/wtf/ParallelHelperPool.h:137:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WTF/wtf/ParallelHelperPool.h:153:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WTF/wtf/ParallelHelperPool.h:173:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WTF/wtf/ParallelHelperPool.cpp:57:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WTF/wtf/ParallelHelperPool.cpp:84:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WTF/wtf/ParallelHelperPool.cpp:107:  Extra space before ( in function call  [whitespace/parens] [4]
Total errors found: 6 in 3 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[David Kilzer (:ddkilzer)]

Comment on attachment 357233 [details]
Patch v1

View in context: https://bugs.webkit.org/attachment.cgi?id=357233&action=review

> Source/WTF/wtf/ParallelHelperPool.cpp:86
>      setTask(task);

Oops!  Forgot the WTFMove() here.

[David Kilzer (:ddkilzer)]

Created attachment 357235 [details]
Patch v1

[EWS Watchlist]

Attachment 357235 [details] did not pass style-queue:


ERROR: Source/WTF/wtf/ParallelHelperPool.h:137:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WTF/wtf/ParallelHelperPool.h:153:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WTF/wtf/ParallelHelperPool.h:173:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WTF/wtf/ParallelHelperPool.cpp:57:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WTF/wtf/ParallelHelperPool.cpp:84:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WTF/wtf/ParallelHelperPool.cpp:107:  Extra space before ( in function call  [whitespace/parens] [4]
Total errors found: 6 in 3 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Alex Christensen]

Comment on attachment 357235 [details]
Patch v1

r=me
Is there a way to see the unnecessary String copies?

[David Kilzer (:ddkilzer)]

(In reply to Alex Christensen from comment #7)
> Comment on attachment 357235 [details]
> Patch v1
> 
> r=me
> Is there a way to see the unnecessary String copies?

In a nutshell:

1. Build your own llvm/clang.
2. Build WebKit and save the full build output to a file.
3. Write a script to take the build output and "replay" the compiler commands with clang-tidy.  (I have a script attached to <rdar://problem/46556354> to do this.  I need to clean it up before landing in the WebKit repo, if the powers that be are okay with that since it requires building your own clang to use, or use a clang distro that includes clang-tidy.)
4. Run the script, then fix the issues it finds.  (I'm doing this now for JavaScriptCore and WebCore.)

[WebKit Commit Bot]

Comment on attachment 357235 [details]
Patch v1

Clearing flags on attachment: 357235

Committed r239177: <https://trac.webkit.org/changeset/239177>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.