> On July 2, 2015, 11:32 p.m., Ben Mahler wrote:
> > src/linux/cgroups.cpp, lines 2014-2027
> > <https://reviews.apache.org/r/36106/diff/1/?file=997647#file997647line2014>
> >
> >     Any reason you're not just re-using cgroups::stat here? I'd suggest 
> > just calling cgroups::stat for now, should simplify this and make it easier 
> > for the reader. :)
> 
> Jojy Varghese wrote:
>     The only reason being that the way cpuacct creates Stat should be 
> encapsulated in the cpuacct::Stat. This is the same reason there is a parse 
> method in Stat. But I can change it to use cgroups::stat if absolutely 
> necessary.
> 
> Jojy Varghese wrote:
>     Also wanted to highlight the semantics of Stat class to make it clear 
> what the intent is. Stat can only be created from data read from the stats 
> file (cpuacct.stat). Thus creating Stat without the file data should not be 
> allowed. Once the Stat object is created, you can not mutate it.

Imagine this only returned the user time, then we would return a Duration, yes? 
We would not be returning a `Stat` class with one const 'user' member, with a 
const getter and a static parse method. That would be overkill, right?

That's the reasoning I have for making this a plain struct. This is the same 
situation as above, but we need to return more than one value (e.g. 
`os::UTSInfo`, `os::Memory`, `os::Load`, etc.)


- Ben


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


On July 2, 2015, 8:09 p.m., Jojy Varghese wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36106/
> -----------------------------------------------------------
> 
> (Updated July 2, 2015, 8:09 p.m.)
> 
> 
> Review request for mesos, Ian Downes, Jie Yu, Joris Van Remoortere, and 
> Timothy Chen.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> cgroups implementation does not have a cpuacct subsystem implementation as of
> today. Adding the implementation for stat function.
> 
> Changes:
>   - added Stats class to encapsulate cpuacct.stat data
>   - added implementation for cpuacct::stats
>   - added unit tests
> 
> Jira: MESOS-2961
> 
> 
> Diffs
> -----
> 
>   src/linux/cgroups.hpp 73b98317880eea3d6a2ba37ac56d1f7e3600ba94 
>   src/linux/cgroups.cpp 4c006d0c7382b940a83359d636c0d48952cdbb00 
>   src/tests/cgroups_tests.cpp 475f48a474eea708f98d8c0300862351a2d4379a 
> 
> Diff: https://reviews.apache.org/r/36106/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>

Reply via email to