-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18173/
-----------------------------------------------------------
Review request for mesos, Benjamin Hindman, Jie Yu, Niklas Nielsen, and Vinod
Kone.
Bugs: MESOS-1008
https://issues.apache.org/jira/browse/MESOS-1008
Repository: mesos-git
Description
-------
See MESOS-1008.
The main goal here is to reduce copying in Try. Most of the code using 'Try's
will either make a copy from t.get(), or use t.get() repeatedly. The latter use
results in many copy operations, one for each call to get().
There is one misuse as a result of this change that will not get caught by the
compiler:
const X& x = try.get().x;
try = foo(); // Now x is garbage.
x.bar(); // Bad!
Fortunately, there were no instances of this in the code.
Similar updates to Option and Result will follow.
Diffs
-----
3rdparty/libprocess/3rdparty/stout/include/stout/try.hpp
6a56c7d175e2e1c0b5f917d8a2d92641d6b7e007
Diff: https://reviews.apache.org/r/18173/diff/
Testing
-------
Updates for libprocess and mesos will come, there were no bugs as a result of
this change, rather only cleanups.
I added the following test to verify the reduction in copies. Interestingly,
gcc-4.8 was eliding the Try copy regardless of whether -O2 was supplied.
I first modified the Try copy constructor:
Try(const Try<T>& that)
{
std::cerr << " Try copy" << std::endl; // Added this.
...
}
struct Foo
{
Foo() : a(1), b(2) {}
Foo(const Foo& f) { a = f.a; b = f.b; std::cerr << " Foo copy" << std::endl;
}
int a, b;
};
Try<Foo> bar() { return Foo(); }
// This test captures the number of copies occurring on stderr.
TEST(TryTest, Copy)
{
Try<Foo> foo = bar();
Foo f1 = foo.get();
const Foo& f2 = foo.get(); // This patch eliminates a copy here.
std::cerr << foo.get().a << std::endl; // This patch eliminates a copy here.
std::cerr << foo.get().b << std::endl; // This patch eliminates a copy here.
}
Current Try:
g++-4.8 with and without O2:
Try<Foo> foo = bar();
Foo copy
Foo f1 = foo.get();
Foo copy
const Foo& f2 = foo.get();
Foo copy
foo.get().a
Foo copy
1
foo.get().b
Foo copy
2
New Try:
g++-4.8 with and without O2:
Try<Foo> foo = bar();
Foo copy
Foo f1 = foo.get();
Foo copy
const Foo& f2 = foo.get();
foo.get().a
1
foo.get().b
2
Thanks,
Ben Mahler
