PSA: Prefer push_back() over emplace_back() unless you know you need it

[Peter Kasting]

TLDR: If you aren't sure you need emplace_back(), use push_back(); emplace_back() is less safe, frequently less readable, and often no faster.

Here's a comparison of the two:

• push_back() invokes copy/move on the supplied object; emplace_back() constructs in place.  However, if you already have a supplied object, this isn't any faster, and if moves are cheap for your type, this isn't very much faster.  In this code:
  
  std::vector<std::unique_ptr<int>> v;
  v.emplace_back(std::make_unique<int>(2));
  
  ...emplace_back() is no faster than push_back(), because both invoke the unique_ptr move constructor.  Thus you can always use push_back() when appending an object from a factory function.  This is a common case in our codebase.
• push_back() refuses to invoke explicit constructors; emplace_back() (by necessity) invokes them.  Therefore it's easy to accidentally invoke the wrong constructor when using emplace_back().  See the Abseil TotW link below.
• Because it omits types, emplace_back() can be mystifying.
  
  v.emplace_back(1, false, "what am I constructing?");
  v.push_back(TestObj(2, true, "ah, a TestObj."));

Our C++ usage page used to contain the following relevant advice:

"When using emplacement for performance reasons, your type should probably be movable (since e.g. a vector of it might be resized); given a movable type, then, consider whether you really need to avoid the move done by push_back(). For readability concerns, treat like auto; sometimes the brevity over push_back() is a win, sometimes a loss. Discussion thread"

A lot of our code uses emplace_back() at best unnecessarily.  Think before you use it blindly.

Further reading:

https://abseil.io/tips/112

http://lists.llvm.org/pipermail/cfe-dev/2016-January/046834.html 

PK

[Lei Zhang]

Can we add this to base/containers/README.md? That'll hopefully give
this PSA more visibility.

> --
> --
> Chromium Developers mailing list: chromi...@chromium.org
> View archives, change email options, or unsubscribe:
> http://groups.google.com/a/chromium.org/group/chromium-dev
> ---
> You received this message because you are subscribed to the Google Groups "Chromium-dev" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to chromium-dev...@chromium.org.
> To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/chromium-dev/CAAHOzFBxh70Q7vkZDA1OCmC2KA4c%2BiPjXPwMeO26XuWGAVJ0Fg%40mail.gmail.com.

[Daniel Cheng]

The linked TotW also talks about preferring emplace() over insert() for std::set... would you say that applies to std::map as well? I personally think emplace() is nicer than insert+std::make_pair for associative containers.

Daniel

You received this message because you are subscribed to the Google Groups "blink-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to blink-dev+...@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CACSHbcQOA4q%3D%2BUC7WctN_VROOcqdmFA%3DB%3Dwv1-E0tyswHWh6Kg%40mail.gmail.com.

[Peter Kasting]

On Wed, Jan 15, 2020 at 6:22 PM Daniel Cheng <dch...@chromium.org> wrote:

The linked TotW also talks about preferring emplace() over insert() for std::set... would you say that applies to std::map as well? I personally think emplace() is nicer than insert+std::make_pair for associative containers.

For clarity (if people don't click through), the TotW argument is that the same considerations for push_back() vs. emplace_back() apply to insert() vs. emplace() (so the conclusion would be "if you don't know you need emplace(), use insert()".

Both insert() and emplace() are uncommon for things like map (where people tend to use [] instead), so the conclusion here is less important.  But where it matters, I'd say that, since you're always inserting pairs, safety of avoiding unintentional implicit construction is unimportant; omitting types doesn't really decrease readability (map.insert(std::make_pair(key, val)) isn't more illuminating because of the explicit call); and there's practically no meaningful speed difference.  Thus you should choose purely on the basis of subjective readability.  For me that leads to this preference order:

With an existing pair |obj|:

map.insert(obj) > map.emplace(obj)

 ["insert" is a clearer word, and "emplace" looks like I'm doing more/different construction than I actually am]

With a |key|, |val|:

map[key] = val > map.emplace(key, val) > map.insert({key, val}) > map.insert(std::make_pair(key, val)) > map.insert(std::pair<KeyT, ValT>(key, val))

[if usable, operator syntax is the most idiomatic; otherwise emplace avoids the most boilerplate noise, and the other options get progressively noisier but not usefully clearer]

PK

[Daniel Cheng]

operator[] syntax definitely looks the most intuitive but has the side effect of default constructing the value if the key doesn't already exist. I doubt that this can typically be optimized out, which would make it suboptimal from a code size perspective (compared to emplace or insert). It is something of a micro-optimization, but I think it would add up if we recommended operator[] over emplace().

Daniel

[Peter Kasting]

On Wed, Jan 15, 2020 at 6:41 PM Daniel Cheng <dch...@chromium.org> wrote:

operator[] syntax definitely looks the most intuitive but has the side effect of default constructing the value if the key doesn't already exist. I doubt that this can typically be optimized out, which would make it suboptimal from a code size perspective (compared to emplace or insert).

Yes, the generated code is noticeably larger, mostly because of the search for the existing element (so that it can be updated if it already exists), which inherently can't be optimized.  The default-construction often _is_ optimized away, but that doesn't save you.  So if you care about code size, this option is worse.

PK

[K. Moon]

An internal TotW (67) used to recommend using map::emplace over map::insert if you want to insert a new value, and operator[] if you want to insert-or-update a value. The TotW now says that since C++17, the general advice in https://abseil.io/tips/112 applies to maps, too.

Given that we're still pre-C++17 in Chromium, I think that implies the advice to use map::emplace over map::insert still holds (dcheng@'s position). I think the internal discussions on this do make a distinction between map and the other container types (since map::emplace always emplaces a pair, not some other type).

--

You received this message because you are subscribed to the Google Groups "blink-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to blink-dev+...@chromium.org.

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CAAHOzFAmbf3a2YxGtGDSh7V3U100b-gisL%3Dnhzo4eVHz%3D0UMRw%40mail.gmail.com.

[Kevin Bailey]

re: [] generates more code, doesn't it have to do the search anyways, to see where to put it? Looking through my local copy, I see it does a lower_bound() as expected.

--

--
Chromium Developers mailing list: chromi...@chromium.org
View archives, change email options, or unsubscribe:
http://groups.google.com/a/chromium.org/group/chromium-dev
---

You received this message because you are subscribed to the Google Groups "Chromium-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to chromium-dev...@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/chromium-dev/CACwGi-7YUYKRcwU%3D5hq3C-JY748%2BQCmcNWwdmDVw58mqmdku-Q%40mail.gmail.com.

[Daniel Cheng]

Right, but the code generated by operator[] also needs to default construct--whereas emplace_back() or push_back() wouldn't. For certain types, this can be surprisingly large.

Daniel

[Yutaka Hirano]

Hi,

I saw a Tricium comment suggesting replacing push_back with emplace_back (for a std::vector) today. Doesn't the comment conflict with this recommendation?

Thanks,

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CAF3XrKogbxL8_Qzo5sYdk5CiO3aPx4R_QFV7%3Dnyw4ptQKRjT5Q%40mail.gmail.com.

[Takuto Ikuta]

+Quinten Yearsley 

On Fri, Aug 14, 2020 at 4:12 PM 'Yutaka Hirano' via Chromium-dev <chromi...@chromium.org> wrote:

Hi,

I saw a Tricium comment suggesting replacing push_back with emplace_back (for a std::vector) today. Doesn't the comment conflict with this recommendation?

I think we want to disable that Tricium rule if the comment is not what we want or confusing?

 

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/chromium-dev/CABihn6FBrxv7w3yQ6q7Pvc94jvPT88DhARGnTeAmxR%3DcsVbnZw%40mail.gmail.com.

[Peter Kasting]

On Fri, Aug 14, 2020 at 12:11 AM Yutaka Hirano <yhi...@google.com> wrote:

Hi,

I saw a Tricium comment suggesting replacing push_back with emplace_back (for a std::vector) today. Doesn't the comment conflict with this recommendation?

In short, not usually; the cases where Tricium suggests emplace_back() are generally ones where it can actually make sense.

That said, note that it's not suggesting

elements_.push_back(DataElement());     ->     elements_.emplace_back(DataElement());

But rather

elements_.push_back(DataElement());     ->     elements_.emplace_back();

PK

[Xianzhu Wang]

FYI crbug.com/1116199 is to improve related rules:

1) to check for emplace_back(ctor(...))

2) to check for emplace_back(constructed_object).

--

You received this message because you are subscribed to the Google Groups "blink-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to blink-dev+...@chromium.org.

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CAAHOzFBnr8O%2BEZf12UuU2tvKgZj9jOHPv4pHXKThbVHcNu7XOQ%40mail.gmail.com.

[Kevin Bailey]

It would be nice to get that recommendation if the type was copy-only.

[Yutaka Hirano]

Thank you for the information, that makes sense.