RESOLVED FIXED 173748
Avoid moving the same vector multiple times 
https://bugs.webkit.org/show_bug.cgi?id=173748
Summary Avoid moving the same vector multiple times
Brent Fulgham
Reported 2017-06-22 16:58:56 PDT
Code inspection revealed that a Vector of String was being WTFMove'd multiple times in a loop. Don't do that!
Attachments
Patch (1.89 KB, patch)
2017-06-22 17:04 PDT, Brent Fulgham 		no flags
DetailsFormatted DiffDiff
Patch (7.86 KB, patch)
2017-06-23 10:15 PDT, Brent Fulgham 		cdumez: review+
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Radar WebKit Bug Importer
Comment 1 2017-06-22 16:59:20 PDT
<rdar://problem/32936804>
Brent Fulgham
Comment 2 2017-06-22 17:04:21 PDT
Created attachment 313675 [details] Patch
Chris Dumez
Comment 3 2017-06-22 18:38:38 PDT
Comment on attachment 313675 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=313675&action=review > Source/WebKit2/UIProcess/WebProcessProxy.cpp:261 > + Vector<String> topDomains = topPrivatelyControlledDomains; Brady or Darin may have a better idea. Personally, I would pass the Vector as a const Vector& until it actually needs copying (inside WebsiteDataStore::fetchDataForTopPrivatelyControlledDomains()).
Brent Fulgham
Comment 4 2017-06-23 09:34:38 PDT
Comment on attachment 313675 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=313675&action=review >> Source/WebKit2/UIProcess/WebProcessProxy.cpp:261 >> + Vector<String> topDomains = topPrivatelyControlledDomains; > > Brady or Darin may have a better idea. Personally, I would pass the Vector as a const Vector& until it actually needs copying (inside WebsiteDataStore::fetchDataForTopPrivatelyControlledDomains()). OK. That will make the patch bigger, but might avoid a number of copies if the lowest level of the 'fetch' algorithm skip this work for some reason. I'll try that.
Brent Fulgham
Comment 5 2017-06-23 10:15:02 PDT
Created attachment 313726 [details] Patch
Chris Dumez
Comment 6 2017-06-23 10:34:55 PDT
Comment on attachment 313726 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=313726&action=review > Source/WebKit2/UIProcess/WebsiteData/WebsiteDataStore.cpp:516 > + for (const auto& topPrivatelyControlledDomain : topPrivatelyControlledDomains) { I think we prefer just "auto&" without const.
Brent Fulgham
Comment 7 2017-06-23 11:05:24 PDT
Committed r218753: <http://trac.webkit.org/changeset/218753>
Note You need to log in before you can comment on or make changes to this bug.