Bug 193498 - clang-tidy: Fix unnecessary copy/ref churn of for loop variables in libwebrtc
Summary: clang-tidy: Fix unnecessary copy/ref churn of for loop variables in libwebrtc
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebRTC (show other bugs)
Version: WebKit Local Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: David Kilzer (:ddkilzer)
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2019-01-16 10:45 PST by David Kilzer (:ddkilzer)
Modified: 2019-01-16 14:45 PST (History)
5 users (show)

See Also:


Attachments
Patch v1 (66.77 KB, patch)
2019-01-16 11:19 PST, David Kilzer (:ddkilzer)
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description David Kilzer (:ddkilzer) 2019-01-16 10:45:37 PST
Running clang-tidy on libwebrtc resulted in these potential performance improvements to prevent object copies or reference churn in for loops:

Source/ThirdParty/libwebrtc/Source/webrtc/rtc_base/filerotatingstream.cc:331:13: warning: loop variable is copied but only used as const reference; consider making it a const reference [performance-for-range-copy]
  for (auto file_name : file_names_) {
            ^
       const  &
--
Source/ThirdParty/libwebrtc/Source/webrtc/pc/jseptransportcontroller.cc:632:15: warning: loop variable is copied but only used as const reference; consider making it a const reference [performance-for-range-copy]
    for (auto content_name : new_bundle_group->content_names()) {
              ^
         const  &
--
Source/ThirdParty/libwebrtc/Source/webrtc/pc/jseptransportcontroller.cc:648:17: warning: loop variable is copied but only used as const reference; consider making it a const reference [performance-for-range-copy]
      for (auto content_name : new_bundle_group->content_names()) {
                ^
           const  &
--
Source/ThirdParty/libwebrtc/Source/webrtc/pc/jseptransportcontroller.cc:659:17: warning: loop variable is copied but only used as const reference; consider making it a const reference [performance-for-range-copy]
      for (auto content_name : bundle_group_->content_names()) {
                ^
           const  &
--
Source/ThirdParty/libwebrtc/Source/webrtc/pc/jseptransportcontroller.cc:707:15: warning: loop variable is copied but only used as const reference; consider making it a const reference [performance-for-range-copy]
    for (auto content_name : bundle_group_->content_names()) {
              ^
         const  &
--
Source/ThirdParty/libwebrtc/Source/webrtc/pc/jseptransportcontroller.cc:742:15: warning: loop variable is copied but only used as const reference; consider making it a const reference [performance-for-range-copy]
    for (auto content_name : bundle_group_->content_names()) {
              ^
         const  &
--
Source/ThirdParty/libwebrtc/Source/webrtc/pc/jseptransportcontroller.cc:848:13: warning: loop variable is copied but only used as const reference; consider making it a const reference [performance-for-range-copy]
  for (auto extension : content_desc->rtp_header_extensions()) {
            ^
       const  &
--
Source/ThirdParty/libwebrtc/Source/webrtc/modules/bitrate_controller/loss_based_bandwidth_estimation.cc:127:13: warning: loop variable is copied but only used as const reference; consider making it a const reference [performance-for-range-copy]
  for (auto pkt : packet_results) {
            ^
       const  &
--
Source/ThirdParty/libwebrtc/Source/webrtc/p2p/base/mdns_message.cc:356:15: warning: loop variable is copied but only used as const reference; consider making it a const reference [performance-for-range-copy]
    for (auto rr : section) {
              ^
         const  &
--
Source/ThirdParty/libwebrtc/Source/webrtc/p2p/base/mdns_message.cc:364:13: warning: loop variable is copied but only used as const reference; consider making it a const reference [performance-for-range-copy]
  for (auto question : question_section_) {
            ^
       const  &
--
Source/ThirdParty/libwebrtc/Source/webrtc/rtc_base/opensslsessioncache.cc:25:13: warning: loop variable is copied but only used as const reference; consider making it a const reference [performance-for-range-copy]
  for (auto it : sessions_) {
            ^
       const  &
--
Source/ThirdParty/libwebrtc/Source/webrtc/pc/peerconnection.cc:846:13: warning: loop variable is copied but only used as const reference; consider making it a const reference [performance-for-range-copy]
  for (auto transceiver : transceivers_) {
            ^
       const  &
--
Source/ThirdParty/libwebrtc/Source/webrtc/pc/peerconnection.cc:882:13: warning: loop variable is copied but only used as const reference; consider making it a const reference [performance-for-range-copy]
  for (auto transceiver : transceivers_) {
            ^
       const  &
--
Source/ThirdParty/libwebrtc/Source/webrtc/pc/peerconnection.cc:887:13: warning: loop variable is copied but only used as const reference; consider making it a const reference [performance-for-range-copy]
  for (auto transceiver : transceivers_) {
            ^
       const  &
--
Source/ThirdParty/libwebrtc/Source/webrtc/pc/peerconnection.cc:1625:13: warning: loop variable is copied but only used as const reference; consider making it a const reference [performance-for-range-copy]
  for (auto sender : GetSendersInternal()) {
            ^
       const  &
--
Source/ThirdParty/libwebrtc/Source/webrtc/pc/peerconnection.cc:1635:13: warning: loop variable is copied but only used as const reference; consider making it a const reference [performance-for-range-copy]
  for (auto transceiver : transceivers_) {
            ^
       const  &
--
Source/ThirdParty/libwebrtc/Source/webrtc/pc/peerconnection.cc:1657:13: warning: loop variable is copied but only used as const reference; consider making it a const reference [performance-for-range-copy]
  for (auto transceiver : transceivers_) {
            ^
       const  &
--
Source/ThirdParty/libwebrtc/Source/webrtc/pc/peerconnection.cc:1670:13: warning: loop variable is copied but only used as const reference; consider making it a const reference [performance-for-range-copy]
  for (auto transceiver : transceivers_) {
            ^
       const  &
--
Source/ThirdParty/libwebrtc/Source/webrtc/pc/peerconnection.cc:1881:13: warning: loop variable is copied but only used as const reference; consider making it a const reference [performance-for-range-copy]
  for (auto transceiver : GetReceivingTransceiversOfType(media_type)) {
            ^
       const  &
--
Source/ThirdParty/libwebrtc/Source/webrtc/pc/peerconnection.cc:1915:13: warning: loop variable is copied but only used as const reference; consider making it a const reference [performance-for-range-copy]
  for (auto transceiver : transceivers_) {
            ^
       const  &
--
Source/ThirdParty/libwebrtc/Source/webrtc/pc/peerconnection.cc:2108:15: warning: loop variable is copied but only used as const reference; consider making it a const reference [performance-for-range-copy]
    for (auto transceiver : transceivers_) {
              ^
         const  &
--
Source/ThirdParty/libwebrtc/Source/webrtc/pc/peerconnection.cc:2136:15: warning: loop variable is copied but only used as const reference; consider making it a const reference [performance-for-range-copy]
    for (auto transceiver : remove_list) {
              ^
         const  &
--
Source/ThirdParty/libwebrtc/Source/webrtc/pc/peerconnection.cc:2139:15: warning: loop variable is copied but only used as const reference; consider making it a const reference [performance-for-range-copy]
    for (auto stream : removed_streams) {
              ^
         const  &
--
Source/ThirdParty/libwebrtc/Source/webrtc/pc/peerconnection.cc:2181:15: warning: loop variable is copied but only used as const reference; consider making it a const reference [performance-for-range-copy]
    for (auto transceiver : transceivers_) {
              ^
         const  &
--
Source/ThirdParty/libwebrtc/Source/webrtc/pc/peerconnection.cc:2461:15: warning: loop variable is copied but only used as const reference; consider making it a const reference [performance-for-range-copy]
    for (auto transceiver : transceivers_) {
              ^
         const  &
--
Source/ThirdParty/libwebrtc/Source/webrtc/pc/peerconnection.cc:2543:15: warning: loop variable is copied but only used as const reference; consider making it a const reference [performance-for-range-copy]
    for (auto transceiver : now_receiving_transceivers) {
              ^
         const  &
--
Source/ThirdParty/libwebrtc/Source/webrtc/pc/peerconnection.cc:2549:15: warning: loop variable is copied but only used as const reference; consider making it a const reference [performance-for-range-copy]
    for (auto stream : added_streams) {
              ^
         const  &
--
Source/ThirdParty/libwebrtc/Source/webrtc/pc/peerconnection.cc:2552:15: warning: loop variable is copied but only used as const reference; consider making it a const reference [performance-for-range-copy]
    for (auto transceiver : remove_list) {
              ^
         const  &
--
Source/ThirdParty/libwebrtc/Source/webrtc/pc/peerconnection.cc:2555:15: warning: loop variable is copied but only used as const reference; consider making it a const reference [performance-for-range-copy]
    for (auto stream : removed_streams) {
              ^
         const  &
--
Source/ThirdParty/libwebrtc/Source/webrtc/pc/peerconnection.cc:2662:13: warning: loop variable is copied but only used as const reference; consider making it a const reference [performance-for-range-copy]
  for (auto stream : media_streams) {
            ^
       const  &
--
Source/ThirdParty/libwebrtc/Source/webrtc/pc/peerconnection.cc:3384:13: warning: loop variable is copied but only used as const reference; consider making it a const reference [performance-for-range-copy]
  for (auto transceiver : transceivers_) {
            ^
       const  &
--
Source/ThirdParty/libwebrtc/Source/webrtc/pc/peerconnection.cc:4047:13: warning: loop variable is copied but only used as const reference; consider making it a const reference [performance-for-range-copy]
  for (auto transceiver : transceivers_) {
            ^
       const  &
--
Source/ThirdParty/libwebrtc/Source/webrtc/pc/peerconnection.cc:4817:13: warning: loop variable is copied but only used as const reference; consider making it a const reference [performance-for-range-copy]
  for (auto transceiver : transceivers_) {
            ^
       const  &
--
Source/ThirdParty/libwebrtc/Source/webrtc/pc/peerconnection.cc:4829:13: warning: loop variable is copied but only used as const reference; consider making it a const reference [performance-for-range-copy]
  for (auto transceiver : transceivers_) {
            ^
       const  &
--
Source/ThirdParty/libwebrtc/Source/webrtc/pc/peerconnection.cc:4841:13: warning: loop variable is copied but only used as const reference; consider making it a const reference [performance-for-range-copy]
  for (auto transceiver : transceivers_) {
            ^
       const  &
--
Source/ThirdParty/libwebrtc/Source/webrtc/pc/peerconnection.cc:5001:13: warning: loop variable is copied but only used as const reference; consider making it a const reference [performance-for-range-copy]
  for (auto transceiver : transceivers_) {
            ^
       const  &
--
Source/ThirdParty/libwebrtc/Source/webrtc/pc/peerconnection.cc:5116:13: warning: loop variable is copied but only used as const reference; consider making it a const reference [performance-for-range-copy]
  for (auto transceiver : transceivers_) {
            ^
       const  &
--
Source/ThirdParty/libwebrtc/Source/webrtc/pc/peerconnection.cc:5453:13: warning: loop variable is copied but only used as const reference; consider making it a const reference [performance-for-range-copy]
  for (auto transceiver : transceivers_) {
            ^
       const  &
--
Source/ThirdParty/libwebrtc/Source/webrtc/pc/peerconnection.cc:5630:13: warning: loop variable is copied but only used as const reference; consider making it a const reference [performance-for-range-copy]
  for (auto transceiver : transceivers_) {
            ^
       const  &
--
Source/ThirdParty/libwebrtc/Source/webrtc/pc/peerconnection.cc:6367:13: warning: loop variable is copied but only used as const reference; consider making it a const reference [performance-for-range-copy]
  for (auto transceiver : transceivers_) {
            ^
       const  &
--
Source/ThirdParty/libwebrtc/Source/webrtc/p2p/base/port.cc:951:13: warning: loop variable is copied but only used as const reference; consider making it a const reference [performance-for-range-copy]
  for (auto kv : connections_) {
            ^
       const  &
--
Source/ThirdParty/libwebrtc/Source/webrtc/video/receive_statistics_proxy.cc:278:13: warning: loop variable is copied but only used as const reference; consider making it a const reference [performance-for-range-copy]
  for (auto it : content_specific_stats_) {
            ^
       const  &
--
Source/ThirdParty/libwebrtc/Source/webrtc/video/receive_statistics_proxy.cc:300:13: warning: loop variable is copied but only used as const reference; consider making it a const reference [performance-for-range-copy]
  for (auto it : aggregated_stats) {
            ^
       const  &
--
Source/ThirdParty/libwebrtc/Source/webrtc/modules/rtp_rtcp/source/rtcp_receiver.cc:440:32: warning: the loop variable's type is not a reference type; this creates a copy in each iteration; consider making this a reference [performance-for-range-copy]
  for (const rtcp::ReportBlock report_block : sender_report.report_blocks())
                               ^
                              &
--
Source/ThirdParty/libwebrtc/Source/webrtc/pc/rtcstatscollector.cc:537:13: warning: loop variable is copied but only used as const reference; consider making it a const reference [performance-for-range-copy]
  for (auto sender : senders) {
            ^
       const  &
--
Source/ThirdParty/libwebrtc/Source/webrtc/pc/rtcstatscollector.cc:605:13: warning: loop variable is copied but only used as const reference; consider making it a const reference [performance-for-range-copy]
  for (auto receiver : receivers) {
            ^
       const  &
--
Source/ThirdParty/libwebrtc/Source/webrtc/pc/rtcstatscollector.cc:1119:15: warning: loop variable is copied but only used as const reference; consider making it a const reference [performance-for-range-copy]
    for (auto sender : stats.transceiver->senders()) {
              ^
         const  &
--
Source/ThirdParty/libwebrtc/Source/webrtc/pc/rtcstatscollector.cc:1127:15: warning: loop variable is copied but only used as const reference; consider making it a const reference [performance-for-range-copy]
    for (auto receiver : stats.transceiver->receivers()) {
              ^
         const  &
--
Source/ThirdParty/libwebrtc/Source/webrtc/pc/rtcstatscollector.cc:1153:15: warning: loop variable is copied but only used as const reference; consider making it a const reference [performance-for-range-copy]
    for (auto sender : stats.transceiver->senders()) {
              ^
         const  &
--
Source/ThirdParty/libwebrtc/Source/webrtc/pc/rtcstatscollector.cc:1160:15: warning: loop variable is copied but only used as const reference; consider making it a const reference [performance-for-range-copy]
    for (auto receiver : stats.transceiver->receivers()) {
              ^
         const  &
--
Source/ThirdParty/libwebrtc/Source/webrtc/pc/rtcstatscollector.cc:1423:13: warning: loop variable is copied but only used as const reference; consider making it a const reference [performance-for-range-copy]
  for (auto transceiver : pc_->GetTransceiversInternal()) {
            ^
       const  &
--
Source/ThirdParty/libwebrtc/Source/webrtc/pc/rtcstatscollector.cc:1496:15: warning: loop variable is copied but only used as const reference; consider making it a const reference [performance-for-range-copy]
    for (auto sender : transceiver->senders()) {
              ^
         const  &
--
Source/ThirdParty/libwebrtc/Source/webrtc/pc/rtcstatscollector.cc:1500:15: warning: loop variable is copied but only used as const reference; consider making it a const reference [performance-for-range-copy]
    for (auto receiver : transceiver->receivers()) {
              ^
         const  &
--
Source/ThirdParty/libwebrtc/Source/webrtc/pc/rtpreceiver.cc:210:13: warning: loop variable is copied but only used as const reference; consider making it a const reference [performance-for-range-copy]
  for (auto existing_stream : streams_) {
            ^
       const  &
--
Source/ThirdParty/libwebrtc/Source/webrtc/pc/rtpreceiver.cc:212:15: warning: loop variable is copied but only used as const reference; consider making it a const reference [performance-for-range-copy]
    for (auto stream : streams) {
              ^
         const  &
--
Source/ThirdParty/libwebrtc/Source/webrtc/pc/rtpreceiver.cc:224:13: warning: loop variable is copied but only used as const reference; consider making it a const reference [performance-for-range-copy]
  for (auto stream : streams) {
            ^
       const  &
--
Source/ThirdParty/libwebrtc/Source/webrtc/pc/rtpreceiver.cc:226:15: warning: loop variable is copied but only used as const reference; consider making it a const reference [performance-for-range-copy]
    for (auto existing_stream : streams_) {
              ^
         const  &
--
Source/ThirdParty/libwebrtc/Source/webrtc/pc/rtpreceiver.cc:409:13: warning: loop variable is copied but only used as const reference; consider making it a const reference [performance-for-range-copy]
  for (auto existing_stream : streams_) {
            ^
       const  &
--
Source/ThirdParty/libwebrtc/Source/webrtc/pc/rtpreceiver.cc:411:15: warning: loop variable is copied but only used as const reference; consider making it a const reference [performance-for-range-copy]
    for (auto stream : streams) {
              ^
         const  &
--
Source/ThirdParty/libwebrtc/Source/webrtc/pc/rtpreceiver.cc:423:13: warning: loop variable is copied but only used as const reference; consider making it a const reference [performance-for-range-copy]
  for (auto stream : streams) {
            ^
       const  &
--
Source/ThirdParty/libwebrtc/Source/webrtc/pc/rtpreceiver.cc:425:15: warning: loop variable is copied but only used as const reference; consider making it a const reference [performance-for-range-copy]
    for (auto existing_stream : streams_) {
              ^
         const  &
--
Source/ThirdParty/libwebrtc/Source/webrtc/pc/rtptransceiver.cc:62:13: warning: loop variable is copied but only used as const reference; consider making it a const reference [performance-for-range-copy]
  for (auto sender : senders_) {
            ^
       const  &
--
Source/ThirdParty/libwebrtc/Source/webrtc/pc/rtptransceiver.cc:67:13: warning: loop variable is copied but only used as const reference; consider making it a const reference [performance-for-range-copy]
  for (auto receiver : receivers_) {
            ^
       const  &
--
Source/ThirdParty/libwebrtc/Source/webrtc/pc/rtptransceiver.cc:150:13: warning: loop variable is copied but only used as const reference; consider making it a const reference [performance-for-range-copy]
  for (auto receiver : receivers_) {
            ^
       const  &
--
Source/ThirdParty/libwebrtc/Source/webrtc/pc/rtptransceiver.cc:215:13: warning: loop variable is copied but only used as const reference; consider making it a const reference [performance-for-range-copy]
  for (auto sender : senders_) {
            ^
       const  &
--
Source/ThirdParty/libwebrtc/Source/webrtc/pc/rtptransceiver.cc:218:13: warning: loop variable is copied but only used as const reference; consider making it a const reference [performance-for-range-copy]
  for (auto receiver : receivers_) {
            ^
       const  &
--
Source/ThirdParty/libwebrtc/Source/webrtc/pc/statscollector.cc:871:13: warning: loop variable is copied but only used as const reference; consider making it a const reference [performance-for-range-copy]
  for (auto transceiver : pc_->GetTransceiversInternal()) {
            ^
       const  &
--
Source/ThirdParty/libwebrtc/Source/webrtc/pc/statscollector.cc:912:15: warning: loop variable is copied but only used as const reference; consider making it a const reference [performance-for-range-copy]
    for (auto transceiver : pc_->GetTransceiversInternal()) {
              ^
         const  &
--
Source/ThirdParty/libwebrtc/Source/webrtc/p2p/base/stunrequest.cc:72:19: warning: the loop variable's type is not a reference type; this creates a copy in each iteration; consider making this a reference [performance-for-range-copy]
  for (const auto kv : requests_) {
                  ^
                 &
--
Source/ThirdParty/libwebrtc/Source/webrtc/p2p/base/stunrequest.cc:82:19: warning: the loop variable's type is not a reference type; this creates a copy in each iteration; consider making this a reference [performance-for-range-copy]
  for (const auto kv : requests_) {
                  ^
                 &
--
Source/ThirdParty/libwebrtc/Source/webrtc/pc/trackmediainfomap.cc:50:13: warning: loop variable is copied but only used as const reference; consider making it a const reference [performance-for-range-copy]
  for (auto rtp_sender : rtp_senders) {
            ^
       const  &
--
Source/ThirdParty/libwebrtc/Source/webrtc/pc/trackmediainfomap.cc:75:13: warning: loop variable is copied but only used as const reference; consider making it a const reference [performance-for-range-copy]
  for (auto rtp_receiver : rtp_receivers) {
            ^
       const  &
--
Source/ThirdParty/libwebrtc/Source/webrtc/pc/trackmediainfomap.cc:129:13: warning: loop variable is copied but only used as const reference; consider making it a const reference [performance-for-range-copy]
  for (auto sender : rtp_senders) {
            ^
       const  &
--
Source/ThirdParty/libwebrtc/Source/webrtc/pc/trackmediainfomap.cc:132:13: warning: loop variable is copied but only used as const reference; consider making it a const reference [performance-for-range-copy]
  for (auto receiver : rtp_receivers) {
            ^
       const  &
Comment 1 David Kilzer (:ddkilzer) 2019-01-16 11:19:50 PST
Created attachment 359283 [details]
Patch v1
Comment 2 youenn fablet 2019-01-16 11:24:41 PST
Comment on attachment 359283 [details]
Patch v1

Can we try upstreaming these changes?

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

> Source/ThirdParty/libwebrtc/Source/webrtc/modules/rtp_rtcp/source/rtcp_receiver.cc:440
> +  for (const rtcp::ReportBlock& report_block : sender_report.report_blocks())

Could use auto here.
Comment 3 David Kilzer (:ddkilzer) 2019-01-16 11:40:35 PST
Comment on attachment 359283 [details]
Patch v1

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

>> Source/ThirdParty/libwebrtc/Source/webrtc/modules/rtp_rtcp/source/rtcp_receiver.cc:440
>> +  for (const rtcp::ReportBlock& report_block : sender_report.report_blocks())
> 
> Could use auto here.

I debated doing that, but went with principle of least change.
Comment 4 Radar WebKit Bug Importer 2019-01-16 14:19:52 PST
<rdar://problem/47329598>
Comment 5 Radar WebKit Bug Importer 2019-01-16 14:19:55 PST
<rdar://problem/47329608>
Comment 6 WebKit Commit Bot 2019-01-16 14:45:00 PST
Comment on attachment 359283 [details]
Patch v1

Clearing flags on attachment: 359283

Committed r240053: <https://trac.webkit.org/changeset/240053>
Comment 7 WebKit Commit Bot 2019-01-16 14:45:02 PST
All reviewed patches have been landed.  Closing bug.