Re: Review Request 44379: Correctly parse perf stat format for non-vanilla 3.10 kernel.

[fan du]

> On 三月 17, 2016, 1:02 a.m., Ben Mahler wrote:
> > src/linux/perf.cpp, lines 435-437
> > 
> >
> > Hm.. this comment is really hard for me to understand, if OS vendors 
> > enhance the format, how did you know that they only use this particular 
> > format and not others?

Thanks for the review!
The format itself doesn't matter, the order of the format does, that's why we 
use size of tokens to parse the format.
Moreover, this format is full fledged which should be supported by all means.

btw, it's unlikely for my custom to depoly upstream kernel in production, 
that's why I stay on Centos7 stable kernel in most cases.


- fan


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


On 三月 10, 2016, 3:26 a.m., fan du wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44379/
> ---
> 
> (Updated 三月 10, 2016, 3:26 a.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Bugs: MESOS-4705
> https://issues.apache.org/jira/browse/MESOS-4705
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> For vanilla kernel <= 3.12, 'perf stat' format is:
>   value,event,cgroup
> Tested with 3.12, 3.11, 3.10 vanilla kernel.
> 
> OS vendors may enhance perf tools with additional format:
>   value,unit,event,cgroup,running,ratio
> Tested on Centos 7 with kernel 3.10.0-{123 OR 229}.el7.x86_64
> 
> 
> Diffs
> -
> 
>   src/linux/perf.cpp 1c113a2b3f57877e132bbd65e01fb2f045132128 
> 
> Diff: https://reviews.apache.org/r/44379/diff/
> 
> 
> Testing
> ---
> 
> 1. {Found and Test} with Serenity, ema filter could get perf event statistics 
> correctly as expected.
> 2. ./bin/mesos-tests.sh --gtest_filter=PerfEventIsolatorTest* 
> --log_dir=/tmp/mesos/
> 
> 
> Thanks,
> 
> fan du
> 
>

Re: Review Request 44379: Correctly parse perf stat format for non-vanilla 3.10 kernel.

[Ben Mahler]

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




src/linux/perf.cpp (lines 435 - 437)


Hm.. this comment is really hard for me to understand, if OS vendors 
enhance the format, how did you know that they only use this particular format 
and not others?


- Ben Mahler


On March 10, 2016, 3:26 a.m., fan du wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44379/
> ---
> 
> (Updated March 10, 2016, 3:26 a.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Bugs: MESOS-4705
> https://issues.apache.org/jira/browse/MESOS-4705
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> For vanilla kernel <= 3.12, 'perf stat' format is:
>   value,event,cgroup
> Tested with 3.12, 3.11, 3.10 vanilla kernel.
> 
> OS vendors may enhance perf tools with additional format:
>   value,unit,event,cgroup,running,ratio
> Tested on Centos 7 with kernel 3.10.0-{123 OR 229}.el7.x86_64
> 
> 
> Diffs
> -
> 
>   src/linux/perf.cpp 1c113a2b3f57877e132bbd65e01fb2f045132128 
> 
> Diff: https://reviews.apache.org/r/44379/diff/
> 
> 
> Testing
> ---
> 
> 1. {Found and Test} with Serenity, ema filter could get perf event statistics 
> correctly as expected.
> 2. ./bin/mesos-tests.sh --gtest_filter=PerfEventIsolatorTest* 
> --log_dir=/tmp/mesos/
> 
> 
> Thanks,
> 
> fan du
> 
>

Re: Review Request 44379: Correctly parse perf stat format for non-vanilla 3.10 kernel.

[Benjamin Mahler]

Hey Fan,

Left some comments, mostly I'm a bit confused by your comment that OS
vendors can tweak the format. How did you know they only tweak it that
particular way?

Ben

On Wed, Mar 9, 2016 at 11:53 PM, Mesos ReviewBot 
wrote:

> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44379/
>
> Patch looks great!
>
> Reviews applied: [44379]
>
> Passed command: export OS='ubuntu:14.04' CONFIGURATION='--verbose' 
> COMPILER='gcc' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; 
> ./support/docker_build.sh
>
>
> - Mesos ReviewBot
>
> On March 10th, 2016, 3:26 a.m. UTC, fan du wrote:
> Review request for mesos and Ben Mahler.
> By fan du.
>
> *Updated March 10, 2016, 3:26 a.m.*
> *Bugs: * MESOS-4705 
> *Repository: * mesos
> Description
>
> For vanilla kernel <= 3.12, 'perf stat' format is:
>   value,event,cgroup
> Tested with 3.12, 3.11, 3.10 vanilla kernel.
>
> OS vendors may enhance perf tools with additional format:
>   value,unit,event,cgroup,running,ratio
> Tested on Centos 7 with kernel 3.10.0-{123 OR 229}.el7.x86_64
>
> Testing
>
>
>1. {Found and Test} with Serenity, ema filter could get perf event 
> statistics correctly as expected.
>2. ./bin/mesos-tests.sh --gtest_filter=PerfEventIsolatorTest* 
> --log_dir=/tmp/mesos/
>
> Diffs
>
>- src/linux/perf.cpp (1c113a2b3f57877e132bbd65e01fb2f045132128)
>
> View Diff 
>

Re: Review Request 44379: Correctly parse perf stat format for non-vanilla 3.10 kernel.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [44379]

Passed command: export OS='ubuntu:14.04' CONFIGURATION='--verbose' 
COMPILER='gcc' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker_build.sh

- Mesos ReviewBot


On March 10, 2016, 3:26 a.m., fan du wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44379/
> ---
> 
> (Updated March 10, 2016, 3:26 a.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Bugs: MESOS-4705
> https://issues.apache.org/jira/browse/MESOS-4705
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> For vanilla kernel <= 3.12, 'perf stat' format is:
>   value,event,cgroup
> Tested with 3.12, 3.11, 3.10 vanilla kernel.
> 
> OS vendors may enhance perf tools with additional format:
>   value,unit,event,cgroup,running,ratio
> Tested on Centos 7 with kernel 3.10.0-{123 OR 229}.el7.x86_64
> 
> 
> Diffs
> -
> 
>   src/linux/perf.cpp 1c113a2b3f57877e132bbd65e01fb2f045132128 
> 
> Diff: https://reviews.apache.org/r/44379/diff/
> 
> 
> Testing
> ---
> 
> 1. {Found and Test} with Serenity, ema filter could get perf event statistics 
> correctly as expected.
> 2. ./bin/mesos-tests.sh --gtest_filter=PerfEventIsolatorTest* 
> --log_dir=/tmp/mesos/
> 
> 
> Thanks,
> 
> fan du
> 
>

Re: Review Request 44379: Correctly parse perf stat format for non-vanilla 3.10 kernel.

[fan du]

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

(Updated 三月 10, 2016, 3:24 a.m.)


Review request for mesos.


Changes
---

Rephrase the commit log and comments to make it clear


Summary (updated)
-

Correctly parse perf stat format for non-vanilla 3.10 kernel.


Bugs: MESOS-4705
https://issues.apache.org/jira/browse/MESOS-4705


Repository: mesos


Description (updated)
---

For vanilla kernel <= 3.12, 'perf stat' format is:
  value,event,cgroup
Tested with 3.12, 3.11, 3.10 vanilla kernel.

OS vendors may enhance perf tools with additional format:
  value,unit,event,cgroup,running,ratio
Tested on Centos 7 with kernel 3.10.0-{123 OR 229}.el7.x86_64


Diffs (updated)
-

  src/linux/perf.cpp 1c113a2b3f57877e132bbd65e01fb2f045132128 

Diff: https://reviews.apache.org/r/44379/diff/


Testing (updated)
---

1. {Found and Test} with Serenity, ema filter could get perf event statistics 
correctly as expected.
2. ./bin/mesos-tests.sh --gtest_filter=PerfEventIsolatorTest* 
--log_dir=/tmp/mesos/


Thanks,

fan du