Reserve memory before using push_back with vector

RESOLVED FIXED in Firefox 57

Status

()

Core
Rewriting and Analysis
RESOLVED FIXED
6 months ago
5 months ago

People

(Reporter: sylvestre, Assigned: sylvestre)

Tracking

(Blocks: 1 bug)

Trunk
mozilla57
Points:
---

Firefox Tracking Flags

(firefox57 fixed)

Details

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(2 attachments)

(Assignee)

Updated

6 months ago
Summary: Reserve memory before using push_back in vector → Reserve memory before using push_back with vector
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)

Comment 3

6 months ago
mozreview-review
Comment on attachment 8893723 [details]
Bug 1387376 - Reserve memory before using push_back with vector

https://reviewboard.mozilla.org/r/164838/#review170392

::: media/webrtc/signaling/src/peerconnection/PeerConnectionMedia.cpp:559
(Diff revision 1)
>      std::vector<std::string> attrs;
> +    attrs.reserve(aCandidateList.size());
>      for (const auto& candidate : aCandidateList) {
>        attrs.push_back("candidate:" + candidate);
>      }
>      attrs.push_back("ice-ufrag:" + aUfrag);
>      attrs.push_back("ice-pwd:" + aPassword);

Should be size() + 2 (see following lines).  r+ with that change
Attachment #8893723 - Flags: review?(rjesup) → review+
Comment hidden (mozreview-request)

Comment 5

5 months ago
mozreview-review
Comment on attachment 8893722 [details]
Bug 1387376 - Reserve memory before using push_back with vector

https://reviewboard.mozilla.org/r/164836/#review170702

It isn't useful to do this in gtests. This code isn't built into firefox and the change will not affect the time we spend running the tests. I don't really care either way. If this is reported by some static analysis and doing this removes noise from the analysis output, then why not. Otherwise, you don't need to bother with fine tuning the performance of loops that are only ever run once in gtests.
Attachment #8893722 - Flags: review?(nical.bugzilla) → review+
(Assignee)

Comment 6

5 months ago
mozreview-review-reply
Comment on attachment 8893722 [details]
Bug 1387376 - Reserve memory before using push_back with vector

https://reviewboard.mozilla.org/r/164836/#review170702

Yeah, this is just to make clang-tidy happy and not bother with ignoring that warning.

Comment 7

5 months ago
Pushed by sledru@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/d0352fcfda33
Reserve memory before using push_back with vector r=nical
https://hg.mozilla.org/integration/autoland/rev/f35d46bca9a9
Reserve memory before using push_back with vector r=jesup

Comment 8

5 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/d0352fcfda33
https://hg.mozilla.org/mozilla-central/rev/f35d46bca9a9
Status: NEW → RESOLVED
Last Resolved: 5 months ago
status-firefox57: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
You need to log in before you can comment on or make changes to this bug.