> On March 11, 2015, 9:38 p.m., Ben Mahler wrote:
> > src/master/http.cpp, line 471
> > <https://reviews.apache.org/r/31700/diff/2/?file=888170#file888170line471>
> >
> > Do you know if a move is helpful here or if the compiler is already
> > optimizing this?
> >
> > ```
> > object.values["slaves"] = std::move(array);
> > ```
> >
> > Don't do it in this change, we need to measure and I'm just curious :)
>
> Alexander Rukletsov wrote:
> I believe the compiler won't optimize this, since `array` is an lvalue
> and may be used after this assignment. One thing how we can try to persuade
> the compiler to move without writing `std::move()` is to wrap array
> population in a function that returns an array, this will be a rvalue then
> and c++11-capable compiler *should* choose the right overload, that takes a
> rvalue ref: http://www.cplusplus.com/reference/map/map/insert/.
>
> But everything more complex than `.reserve()` should be benchmarked, on
> all official supported compilers if possible. I'll play with this when I have
> some free cycles.
>
> Cody Maloney wrote:
> From writing my own JSON code, the std::move here is definitely necessary
> to do this as quickly and cheaply as possible.
>
> Using std::move here I think is actually the right thing to do, rather
> than trying to convince the compiler via other means, it's explicitly saying
> to the reader "This variable you were using earlier, it is having it's
> contents ripped out of it".
>
> Putting it into a function and guaranteeing NRVO or RVO fire so that you
> get the "cheap" move insertion has a lot of things people can disturb only
> slightly and end up breaking it.
>
> Cody Maloney wrote:
> Updating the instance in <stout/protobuf.hpp> where the JSON::Array is
> copied to be a move would also probably help significantly.
>
> The way JSON::Protobuf's api is setup with Operator() forces a copy of
> the JSON::Object after it is fully constructed which is rather expensive.
> Would be good to make JSON::Protobuf() just be a function which can then use
> the object internally, and move out the result rather than forcing the full
> object copy.
TL;DR: `std::move` will help.
### Does the compiler see that the `array` variable can be moved? No.
It looks like the compiler could probably perform a NRVO-like optimization
here, but it doesn't.
```cpp
#include <iostream>
class Foo {
public:
Foo() { std::cout << "Foo()" << std::endl; }
Foo(const Foo &) { std::cout << "Foo(const Foo &)" << std::endl; }
Foo(Foo &&) noexcept { std::cout << "Foo(Foo &&)" << std::endl; }
Foo &operator=(const Foo &) {
std::cout << "Foo &operator=(const Foo &)" << std::endl;
return *this;
}
Foo &operator=(Foo &&) noexcept {
std::cout << "Foo &operator=(Foo &&)" << std::endl;
return *this;
}
};
int main() {
Foo result;
{
Foo x;
result = x;
}
{
Foo y;
result = std::move(y);
}
}
```
The above prints:
```bash
Foo()
Foo()
Foo &operator=(const Foo &)
Foo()
Foo &operator=(Foo &&)
```
That is compiled with `clang++ -std=c++14 -O2 a.cc`. It's safe to say that if
can't optimize that obvious-looking case, ours won't be optimized either.
- Michael
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31700/#review76133
-----------------------------------------------------------
On March 11, 2015, 10:40 p.m., Alexander Rukletsov wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31700/
> -----------------------------------------------------------
>
> (Updated March 11, 2015, 10:40 p.m.)
>
>
> Review request for mesos and Ben Mahler.
>
>
> Bugs: MESOS-2353
> https://issues.apache.org/jira/browse/MESOS-2353
>
>
> Repository: mesos
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> src/common/http.cpp 0b57fb01f7769031704e5341849bf95a0197ffd9
> src/master/http.cpp 0b56cb419b0e488eb4163739f57ee1837ca83d24
>
> Diff: https://reviews.apache.org/r/31700/diff/
>
>
> Testing
> -------
>
> make check (OS X 10.9.5, Ubuntu 14.04)
>
>
> Thanks,
>
> Alexander Rukletsov
>
>
