Bug 202069

Summary: clang-tidy: Fix unnecessary copy/ref churn of for loop variables in WTF/JavaScriptCore
Product: WebKit Reporter: David Kilzer (:ddkilzer) <ddkilzer>
Component: JavaScriptCoreAssignee: David Kilzer (:ddkilzer) <ddkilzer>
Status: RESOLVED FIXED    
Severity: Normal CC: benjamin, cdumez, cmarcelo, commit-queue, dbates, ews-watchlist, hi, joepeck, keith_miller, mark.lam, msaboff, saam, tzagallo, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=202090
https://bugs.webkit.org/show_bug.cgi?id=202096
Attachments:
Description Flags
Patch v1
none
Patch v2 none

Description David Kilzer (:ddkilzer) 2019-09-20 19:21:50 PDT
Running clang-tidy on WTF and JavaScriptCore resulted in these potential performance improvements to prevent object copies or reference churn in for loop variables:

Source/WTF/wtf/AggregateLogger.h:107:19: warning: loop variable is copied but only used as const reference; consider making it a const reference [performance-for-range-copy]
        for (auto logger : m_loggers) {
                  ^
             const  &
--
Source/JavaScriptCore/parser/Parser.h:333:23: warning: loop variable is copied but only used as const reference; consider making it a const reference [performance-for-range-copy]
            for (auto entry : m_lexicalVariables) {
                      ^
                 const  &
--
Source/JavaScriptCore/./inspector/remote/cocoa/RemoteInspectorCocoa.mm:242:15: warning: loop variable is copied but only used as const reference; consider making it a const reference [performance-for-range-copy]
    for (auto targetConnection : m_targetConnectionMap.values())
              ^
         const  &
--
Source/JavaScriptCore/./inspector/remote/cocoa/RemoteInspectorCocoa.mm:350:15: warning: loop variable is copied but only used as const reference; consider making it a const reference [performance-for-range-copy]
    for (auto targetConnection : m_targetConnectionMap.values())
              ^
         const  &
--
Source/JavaScriptCore/./inspector/remote/cocoa/RemoteInspectorCocoa.mm:440:34: warning: loop variable is copied but only used as const reference; consider making it a const reference [performance-for-range-copy]
    for (RetainPtr<NSDictionary> listing : m_targetListingMap.values()) {
                                 ^
         const                  &
--
Source/JavaScriptCore/./bytecode/CodeBlock.cpp:884:19: warning: loop variable is copied but only used as const reference; consider making it a const reference [performance-for-range-copy]
        for (auto setEntry : set) {
                  ^
             const  &
--
Source/JavaScriptCore/./bytecompiler/BytecodeGenerator.cpp:2316:19: warning: loop variable is copied but only used as const reference; consider making it a const reference [performance-for-range-copy]
        for (auto pair : activationValuesToCopyOver) {
                  ^
             const  &
--
Source/JavaScriptCore/./dfg/DFGGraph.cpp:549:19: warning: loop variable is copied but only used as const reference; consider making it a const reference [performance-for-range-copy]
        for (auto pair : m_rootToArguments)
                  ^
             const  &
--
Source/JavaScriptCore/./inspector/agents/InspectorAgent.cpp:123:15: warning: loop variable is copied but only used as const reference; consider making it a const reference [performance-for-range-copy]
    for (auto domainName : extraDomains)
              ^
         const  &
--
Source/JavaScriptCore/./runtime/ProxyObject.cpp:1059:19: warning: loop variable is copied but only used as const reference; consider making it a const reference [performance-for-range-copy]
        for (auto propertyName : trapResult) {
                  ^
             const  &
--
Source/JavaScriptCore/./runtime/ProxyObject.cpp:1070:19: warning: loop variable is copied but only used as const reference; consider making it a const reference [performance-for-range-copy]
        for (auto propertyName : trapResult)
                  ^
             const  &
--
Source/JavaScriptCore/./runtime/SamplingProfiler.cpp:1016:23: warning: loop variable is copied but only used as const reference; consider making it a const reference [performance-for-range-copy]
            for (auto profile : *profilesToReport)
                      ^
                 const  &
--
Source/JavaScriptCore/./runtime/SamplingProfiler.cpp:1069:19: warning: loop variable is copied but only used as const reference; consider making it a const reference [performance-for-range-copy]
        for (auto entry : functionCounts) {
                  ^
             const  &
--
Source/JavaScriptCore/./runtime/SamplingProfiler.cpp:1149:19: warning: loop variable is copied but only used as const reference; consider making it a const reference [performance-for-range-copy]
        for (auto entry : bytecodeCounts) {
                  ^
             const  &
--
Source/JavaScriptCore/./runtime/TypeSet.cpp:500:19: warning: loop variable is copied but only used as const reference; consider making it a const reference [performance-for-range-copy]
        for (auto field : currentShape->m_fields)
                  ^
             const  &
--
Source/JavaScriptCore/./runtime/TypeSet.cpp:502:19: warning: loop variable is copied but only used as const reference; consider making it a const reference [performance-for-range-copy]
        for (auto field : currentShape->m_optionalFields)
                  ^
             const  &
--
Source/JavaScriptCore/./runtime/TypeSet.cpp:541:15: warning: loop variable is copied but only used as const reference; consider making it a const reference [performance-for-range-copy]
    for (auto field : a->m_fields) {
              ^
         const  &
--
Source/JavaScriptCore/./runtime/TypeSet.cpp:548:15: warning: loop variable is copied but only used as const reference; consider making it a const reference [performance-for-range-copy]
    for (auto field : b->m_fields) {
              ^
         const  &
--
Source/JavaScriptCore/./runtime/TypeSet.cpp:555:15: warning: loop variable is copied but only used as const reference; consider making it a const reference [performance-for-range-copy]
    for (auto field : a->m_optionalFields)
              ^
         const  &
--
Source/JavaScriptCore/./runtime/TypeSet.cpp:557:15: warning: loop variable is copied but only used as const reference; consider making it a const reference [performance-for-range-copy]
    for (auto field : b->m_optionalFields)
              ^
         const  &
Comment 1 David Kilzer (:ddkilzer) 2019-09-20 19:25:49 PDT
Created attachment 379302 [details]
Patch v1
Comment 2 Radar WebKit Bug Importer 2019-09-20 19:26:01 PDT
<rdar://problem/55580520>
Comment 3 Radar WebKit Bug Importer 2019-09-20 19:26:01 PDT
<rdar://problem/55580521>
Comment 4 David Kilzer (:ddkilzer) 2019-09-20 19:44:53 PDT
Created attachment 379303 [details]
Patch v2
Comment 5 David Kilzer (:ddkilzer) 2019-09-20 19:45:24 PDT
Comment on attachment 379302 [details]
Patch v1

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

> Source/JavaScriptCore/runtime/TypeSet.cpp:557
> +    for (cont auto& field : b->m_optionalFields)

Typo:  cont auto&
Comment 6 Mark Lam 2019-09-20 20:33:35 PDT
Comment on attachment 379303 [details]
Patch v2

r=me
Comment 7 WebKit Commit Bot 2019-09-21 04:40:04 PDT
Comment on attachment 379303 [details]
Patch v2

Clearing flags on attachment: 379303

Committed r250180: <https://trac.webkit.org/changeset/250180>
Comment 8 WebKit Commit Bot 2019-09-21 04:40:06 PDT
All reviewed patches have been landed.  Closing bug.