> On Oct. 23, 2014, 7:46 p.m., Vinod Kone wrote: > > Why are some definitions in .hpp and some in .cpp? Why not all in .cpp? > > > > Also, it's not clear to me how this split would help in unit testing? > > AFAICT, all these visitors take Master or Framework or Slave which needs > > bringing up a cluster. > > Dominic Hamon wrote: > They could be all in cpp, but some are so simple that being inlineable > seemed beneficial. > > This is a first step. The Master/Slave/Frameworks passed in could be > mock/stub versions that would support lightweight testing. The change also > has a benefit in reducing the amount of code in the master source file, which > helps with compile time and readability.
Things in cpp files can be inlined with LTO (Which is something I'd like to get functioning in Mesos tooling in in the long term). There isn't much code which includes the header, so the increase in compile time / object size from having more in the header isn't something I'm worried about in this case though, so net "meh" from me either way. Reducing code in master.cpp is definitely good, although most of the slowness compiling it comes from things it includes, not master.cpp itself. (Flags tends to be one of the worse offenders last time I did some investigation). - Cody ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/20423/#review58071 ----------------------------------------------------------- On Oct. 23, 2014, 10:48 p.m., Dominic Hamon wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/20423/ > ----------------------------------------------------------- > > (Updated Oct. 23, 2014, 10:48 p.m.) > > > Review request for mesos and Benjamin Hindman. > > > Bugs: MESOS-1064 > https://issues.apache.org/jira/browse/MESOS-1064 > > > Repository: mesos-git > > > Description > ------- > > This is the first step toward being able to write independent unit tests for > the validation visitors. > > It also uses Owned to make visitor cleanup simpler (non-existent). > > > Diffs > ----- > > 3rdparty/libprocess/include/process/timeout.hpp > 0bf63e11a7a63b714aafb5386bf2d169260396ce > src/Makefile.am 2617f77b757cb7414889520c88b1bc203dedef09 > src/master/master.cpp 95589b8f25a3e496eabc0cf84319c883c1ed7aec > src/master/offervisitor.hpp PRE-CREATION > src/master/offervisitor.cpp PRE-CREATION > src/master/taskinfovisitor.hpp PRE-CREATION > src/master/taskinfovisitor.cpp PRE-CREATION > src/slave/containerizer/mesos/containerizer.cpp > 9f745d897119a814bd9f8e1b6a0ce5eaef60ed36 > src/zookeeper/group.hpp 924613065521a72887a2721b3db89f448fa50900 > > Diff: https://reviews.apache.org/r/20423/diff/ > > > Testing > ------- > > make check. > ran Java test framework. > > > Thanks, > > Dominic Hamon > >
