> On Oct. 9, 2014, 2:05 p.m., Dominic Hamon wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/option.hpp, line 131
> > <https://reviews.apache.org/r/26476/diff/1/?file=716336#file716336line131>
> >
> >     std::unique_ptr would also be an option as it can be moved on Option 
> > copy. Would that be a less intrusive change?

That would effectively make Option a pointer to a pointer... Better would be to 
do something like:
```
class Option : std::unique_ptr {
 ...
}
```

But overall making the optional thing live in place is preferable. In the mesos 
codebase there aren't any really large objects (Objects with a lot of members, 
or large in-place array members).

Anything that grows / tends to get really large (We move about some big 
strings), all have external storage from the base class, and this object stays 
small.

In terms of perf, linear copies of an object in place are fast, and practically 
copying around the object a whole lot is waayyy faster than if we have a single 
cache miss looking up the pointer. If you want a real world example of this, 
std::vector is almost always faster than std::list, unless you have very large 
objects 
(http://baptiste-wicht.com/posts/2012/12/cpp-benchmark-vector-list-deque.html)


- Cody


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26476/#review55977
-----------------------------------------------------------


On Oct. 9, 2014, 1:35 a.m., Joris Van Remoortere wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26476/
> -----------------------------------------------------------
> 
> (Updated Oct. 9, 2014, 1:35 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Niklas Nielsen.
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> Remove dynamic allocations from Option class.
> 
> 
> Diffs
> -----
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/option.hpp 47fe92c 
> 
> Diff: https://reviews.apache.org/r/26476/diff/
> 
> 
> Testing
> -------
> 
> make check
> support/mesos-style.py
> valgrind (reduced allocation count)
> 
> 
> Thanks,
> 
> Joris Van Remoortere
> 
>

Reply via email to