You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mesos.apache.org by Ian Downes <ia...@gmail.com> on 2014/06/03 22:45:06 UTC
Re: Review Request 21443: Support for running `perf stat`.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/21443/
-----------------------------------------------------------
(Updated June 3, 2014, 1:45 p.m.)
Review request for mesos, Ben Mahler and Vinod Kone.
Changes
-------
Cleaned up the API per Ben's suggestions
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 (updated)
-----
src/Makefile.am ffde59be8683dd40cc5bc7cb88cd88c5bc91cf96
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
Re: Review Request 21443: Support for running `perf stat`.
Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/21443/#review45662
-----------------------------------------------------------
Ship it!
Ship It!
- Vinod Kone
On June 13, 2014, 9:39 p.m., Ian Downes wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/21443/
> -----------------------------------------------------------
>
> (Updated June 13, 2014, 9:39 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 c91b438c3401ff815116d6faab4f0a34bc0fe3fd
> 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
>
>
Re: Review Request 21443: Support for running `perf stat`.
Posted by Ian Downes <ia...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/21443/
-----------------------------------------------------------
(Updated June 13, 2014, 2:39 p.m.)
Review request for mesos and Vinod Kone.
Changes
-------
Addressed Vinod's comments.
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 (updated)
-----
src/Makefile.am c91b438c3401ff815116d6faab4f0a34bc0fe3fd
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
Re: Review Request 21443: Support for running `perf stat`.
Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
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
>
>
Re: Review Request 21443: Support for running `perf stat`.
Posted by Mesos ReviewBot <de...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/21443/#review45588
-----------------------------------------------------------
Bad patch!
Reviews applied: [21443]
Failed command: git apply --index 21443.patch
Error:
error: patch failed: src/Makefile.am:287
error: src/Makefile.am: patch does not apply
- Mesos ReviewBot
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
>
>
Re: Review Request 21443: Support for running `perf stat`.
Posted by Ian Downes <ia...@gmail.com>.
-----------------------------------------------------------
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.
Changes
-------
i'll shepherd this -- vinod.
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
Re: Review Request 21443: Support for running `perf stat`.
Posted by Mesos ReviewBot <de...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/21443/#review45441
-----------------------------------------------------------
Bad patch!
Reviews applied: [21442, 21443]
Failed command: git apply --index 21443.patch
Error:
error: patch failed: src/Makefile.am:287
error: src/Makefile.am: patch does not apply
- Mesos ReviewBot
On June 11, 2014, 5:29 p.m., Ian Downes wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/21443/
> -----------------------------------------------------------
>
> (Updated June 11, 2014, 5:29 p.m.)
>
>
> Review request for mesos, Ben Mahler 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
>
>
Re: Review Request 21443: Support for running `perf stat`.
Posted by Ian Downes <ia...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/21443/
-----------------------------------------------------------
(Updated June 11, 2014, 10:29 a.m.)
Review request for mesos, Ben Mahler and Vinod Kone.
Changes
-------
Added back depends on.
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
Re: Review Request 21443: Support for running `perf stat`.
Posted by Mesos ReviewBot <de...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/21443/#review45065
-----------------------------------------------------------
Bad patch!
Reviews applied: [21443]
Failed command: git apply --index 21443.patch
Error:
error: patch failed: src/Makefile.am:287
error: src/Makefile.am: patch does not apply
- Mesos ReviewBot
On June 9, 2014, 5:09 a.m., Ian Downes wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/21443/
> -----------------------------------------------------------
>
> (Updated June 9, 2014, 5:09 a.m.)
>
>
> Review request for mesos, Ben Mahler 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
>
>
Re: Review Request 21443: Support for running `perf stat`.
Posted by Ian Downes <ia...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/21443/
-----------------------------------------------------------
(Updated June 8, 2014, 10:09 p.m.)
Review request for mesos, Ben Mahler and Vinod Kone.
Changes
-------
Rebased and minor clean ups.
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 (updated)
-----
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
Re: Review Request 21443: Support for running `perf stat`.
Posted by Ian Downes <ia...@gmail.com>.
> On June 6, 2014, 12:14 p.m., Vinod Kone wrote:
> > src/linux/perf.cpp, line 332
> > <https://reviews.apache.org/r/21443/diff/2-5/?file=581929#file581929line332>
> >
> > s/use ""/use "" as key/ ?
> >
> > also, i don't understand the semantics of ""? why is this not an Option?
Option<T> isn't hashable so it uses "" when it's for pids, rather than the cgroup name.
> On June 6, 2014, 12:14 p.m., Vinod Kone wrote:
> > src/linux/perf.cpp, line 433
> > <https://reviews.apache.org/r/21443/diff/2-5/?file=581929#file581929line433>
> >
> > can this be pid or stringify(pids) instead of "" ?
No, it needs to be something known that can't match any cgroup name.
- Ian
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/21443/#review44951
-----------------------------------------------------------
On June 3, 2014, 1:45 p.m., Ian Downes wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/21443/
> -----------------------------------------------------------
>
> (Updated June 3, 2014, 1:45 p.m.)
>
>
> Review request for mesos, Ben Mahler 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 ffde59be8683dd40cc5bc7cb88cd88c5bc91cf96
> 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
>
>
Re: Review Request 21443: Support for running `perf stat`.
Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/21443/#review44951
-----------------------------------------------------------
src/linux/perf.hpp
<https://reviews.apache.org/r/21443/#comment79534>
s/Expose/NOTE: Exposed/
src/linux/perf.cpp
<https://reviews.apache.org/r/21443/#comment79540>
s/,/;/
src/linux/perf.cpp
<https://reviews.apache.org/r/21443/#comment79541>
s/use ""/use "" as key/ ?
also, i don't understand the semantics of ""? why is this not an Option?
src/linux/perf.cpp
<https://reviews.apache.org/r/21443/#comment79539>
Taking a future as an argument here seems weird. Why not take the hashmap directly?
AFAICT, this is called with a .then().
src/linux/perf.cpp
<https://reviews.apache.org/r/21443/#comment79542>
can this be pid or stringify(pids) instead of "" ?
- Vinod Kone
On June 3, 2014, 8:45 p.m., Ian Downes wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/21443/
> -----------------------------------------------------------
>
> (Updated June 3, 2014, 8:45 p.m.)
>
>
> Review request for mesos, Ben Mahler 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 ffde59be8683dd40cc5bc7cb88cd88c5bc91cf96
> 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
>
>