Bug 192666

Summary: clang-tidy: Fix unnecessary parameter copies in ParallelHelperPool.cpp
Product: WebKit Reporter: David Kilzer (:ddkilzer) <ddkilzer>
Component: Web Template FrameworkAssignee: David Kilzer (:ddkilzer) <ddkilzer>
Status: RESOLVED FIXED    
Severity: Normal CC: achristensen, benjamin, cdumez, cmarcelo, commit-queue, darin, dbates, ews-watchlist, fpizlo, ggaren, mark.lam, webkit-bug-importer, ysuzuki
Priority: P2 Keywords: InRadar
Version: WebKit Local Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch v1
none
Patch v1 none

David Kilzer (:ddkilzer)
Reported 2018-12-13 08:37:07 PST
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 &
Attachments
Patch v1 (4.11 KB, patch)
2018-12-13 08:46 PST, David Kilzer (:ddkilzer) 		no flags
DetailsFormatted DiffDiff
Patch v1 (4.09 KB, patch)
2018-12-13 09:11 PST, David Kilzer (:ddkilzer) 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Radar WebKit Bug Importer
Comment 1 2018-12-13 08:37:48 PST
<rdar://problem/46697952>
David Kilzer (:ddkilzer)
Comment 2 2018-12-13 08:46:41 PST
Created attachment 357233 [details] Patch v1
EWS Watchlist
Comment 3 2018-12-13 08:48:33 PST
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 4 2018-12-13 09:10:58 PST
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)
Comment 5 2018-12-13 09:11:32 PST
Created attachment 357235 [details] Patch v1
EWS Watchlist
Comment 6 2018-12-13 09:13:00 PST
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 7 2018-12-13 11:19:33 PST
Comment on attachment 357235 [details] Patch v1 r=me Is there a way to see the unnecessary String copies?
David Kilzer (:ddkilzer)
Comment 8 2018-12-13 12:15:51 PST
(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 9 2018-12-13 12:49:19 PST
Comment on attachment 357235 [details] Patch v1 Clearing flags on attachment: 357235 Committed r239177: <https://trac.webkit.org/changeset/239177>
WebKit Commit Bot
Comment 10 2018-12-13 12:49:21 PST
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.