broadinstitute / gamgee

A C++14 library for NGS data formats
http://broadinstitute.github.io/gamgee/
MIT License
40 stars 13 forks source link

Expunge copy-then-move idiom from gamgee/foghorn #256

Closed droazen closed 9 years ago

droazen commented 9 years ago

I think that Herb Sutter pretty much destroyed the arguments for the copy-then-move idiom at CPPCon -- let's stop using/recommending this. Major counterarguments are:

-can't re-use existing capacity in collections (since you're allocating a new object at call time rather than assignment time)

-making functions noexcept becomes problematic, since the function will compile only because the operation that can throw (copying) has been moved to the caller

droazen commented 9 years ago

To clarify, this is the idiom in which you take an argument by value, then move-assign it in the function body. Herb's performance testing results showing the huge spike in runtime when using this idiom speak for themselves.

MauricioCarneiro commented 9 years ago

he also said that it is good to use this in constructors with multiple parameters (which as far as I can remember was the only few use cases in our repo). But sure, this issue is valuable. We should make sure that only those cases constructors with multiple parameters benefit from that optimization.

droazen commented 9 years ago

He said it was "ok" in that case, not necessarily good, and ONLY if you care about micro-optimizing for r-values AND want to avoid writing combinatorial overloads.

Given that this is a very specific exception, and given the serious problems this idiom can cause, I vote that we just ban it entirely.

MauricioCarneiro commented 9 years ago

I suggest you ask Scott Meyer's opinion on this one monday :)

droazen commented 9 years ago

Ah, you know very well that Scott and Herb have a difference of opinion here :)

droazen commented 9 years ago

Let the record state that Mauricio & I cornered Scott after his talk at the Broad today and asked him about this issue, and that it turns out he agrees that this copy-then-move idiom is bad in most cases. He would allow for exceptions in rare cases such as functions with many parameters where you don't want to write tons of overloads, however, which is consistent with Herb Sutter's CPPCon talk.

MauricioCarneiro commented 9 years ago

True that. I'm sold.