1387376 - Reserve memory before using push_back with vector

Status: RESOLVED FIXED

(Developer Infrastructure :: Source Code Analysis, enhancement)

Description

[Sylvestre Ledru [:Sylvestre]]

https://clang.llvm.org/extra/clang-tidy/checks/performance-inefficient-vector-operation.html

Comment 1

[Sylvestre Ledru [:Sylvestre]]

Attachment: Bug 1387376 - Reserve memory before using push_back with vector

Review commit: https://reviewboard.mozilla.org/r/164836/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/164836/

Comment 2

[Sylvestre Ledru [:Sylvestre]]

Attachment: Bug 1387376 - Reserve memory before using push_back with vector

Review commit: https://reviewboard.mozilla.org/r/164838/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/164838/

Comment 3

[Randell Jesup [:jesup] (needinfo me)]

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

Comment 4

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8893723 [details]
Bug 1387376 - Reserve memory before using push_back with vector

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/164838/diff/1-2/

Comment 5

[Nicolas Silva [:nical]]

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.

Comment 6

[Sylvestre Ledru [:Sylvestre]]

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

[Pulsebot]

Pushed by [e-mail]:
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

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/d0352fcfda33
https://hg.mozilla.org/mozilla-central/rev/f35d46bca9a9