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



src/linux/perf.cpp
<https://reviews.apache.org/r/21443/#comment80439>

    const?



src/linux/perf.cpp
<https://reviews.apache.org/r/21443/#comment80444>

    i still think an empty string is a bit weird. you didn't want to use 
something like "__pids__" because a cgroup can have that name? Mesos 
conainerizer uses UUIDs so it shouldn't be a problem, but I guess EC can create 
arbitrary cgroup names? can a cgroup name be ""?
    
    either way, extract this into a constant (PIDS_KEY?).



src/linux/perf.cpp
<https://reviews.apache.org/r/21443/#comment80440>

    const?


- Vinod Kone


On June 12, 2014, 9:51 p.m., Ian Downes wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/21443/
> -----------------------------------------------------------
> 
> (Updated June 12, 2014, 9:51 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-1278
>     https://issues.apache.org/jira/browse/MESOS-1278
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> Perf can be run against either a set of pids or a perf_event cgroup.
> 
> I decided to parse the perf output into the protobuf directly, rather than 
> via JSON, because it was simpler to handle output like <not supported> and 
> <not counted>. Plus JSON's floating point numbers cannot fully represent 
> uint64.
> 
> 
> Diffs
> -----
> 
>   src/Makefile.am 4a3f2e12643a1f02284587ebcbe9374f416b3d60 
>   src/linux/perf.hpp PRE-CREATION 
>   src/linux/perf.cpp PRE-CREATION 
>   src/tests/cgroups_tests.cpp 5f674cd678e67f10bfef4620d927bb5af7c93753 
>   src/tests/perf_tests.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/21443/diff/
> 
> 
> Testing
> -------
> 
> Added tests for pids and cgroups.
> 
> 
> Thanks,
> 
> Ian Downes
> 
>

Reply via email to