> On Oct. 9, 2014, 3:45 a.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> > > > > This makes Option arbitrarily large which could be problematic where we > > copy it (we can't assume move semantics). > > > > I don't understand the benefit of this change. We have so many dynamic > > allocations throughout the code-base, it seems like a strange place to > > focus attention.
In the original implementation of Option, a large T would still be deep copied; it would just be done on the heap. To avoid large copies one should use a reference counted abstraction such as shared_ptr (e.g. Option<std::shared_ptr<T>> or std::shared_ptr<Option<T>>). Option is meant to augment a type with 1 extra bit of (nullable / unknownable, whichever you prefer) state. Tackling Option is one way of reducing a significant number of dynamic allocations as it is a heavily used library. Option is also something that is commonly assumed to be a light-weight abstraction; so it is less of a surprise if it doesn't have an underlying dynamic allocation. - Joris ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26476/#review55945 ----------------------------------------------------------- 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 > >
