Re: Review Request 37541: Add trace event API

[Cong Wang]

> On Nov. 24, 2015, 12:53 a.m., Vinod Kone wrote:
> > src/linux/perf.hpp, lines 108-110
> > 
> >
> > why are these public?

Easy to access, instead of adding a get/set for each of them.


> On Nov. 24, 2015, 12:53 a.m., Vinod Kone wrote:
> > src/linux/perf.cpp, lines 1125-1128
> > 
> >
> > this could use some comments.

What comments do you expect here? Some comment to explain what ID is? I thought 
the code is clear.


> On Nov. 24, 2015, 12:53 a.m., Vinod Kone wrote:
> > src/tests/containerizer/cgroups_tests.cpp, line 1105
> > 
> >
> > no need for this?

We need this, otherwise we would still hold a ref to the cgroup which causes a 
failure to destroy it later.


> On Nov. 24, 2015, 12:53 a.m., Vinod Kone wrote:
> > src/linux/perf.cpp, lines 974-1029
> > 
> >
> > this could use some comments.

I will add some ASCII arts here to draw the ring buffer.


- Cong


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


On Sept. 30, 2015, 12:14 a.m., Cong Wang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37541/
> ---
> 
> (Updated Sept. 30, 2015, 12:14 a.m.)
> 
> 
> Review request for mesos, Chi Zhang, Ian Downes, Jie Yu, and Vinod Kone.
> 
> 
> Bugs: MESOS-2769
> https://issues.apache.org/jira/browse/MESOS-2769
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Based on the PerfEvent API's, add API for Linux kernel trace events, 
> especially the schedule trace events.
> 
> 
> Diffs
> -
> 
>   src/linux/perf.hpp d10968ca670eb516bae08385fd0ddde8e8ad83b5 
>   src/linux/perf.cpp f7035ddb2507a7646d88dd517d048018f695448a 
>   src/tests/containerizer/cgroups_tests.cpp 
> 75a3bc0009c037dc18ce319db2eb44630f083e8c 
>   src/tests/containerizer/perf_tests.cpp 
> ed5212ee31b8aa47339b8b8fab184bbdf85be82a 
> 
> Diff: https://reviews.apache.org/r/37541/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Cong Wang
> 
>

Re: Review Request 37541: Add trace event API

[Vinod Kone]

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



src/linux/perf.hpp (line 95)


s/inline//



src/linux/perf.hpp (line 100)


s/inline//



src/linux/perf.hpp (line 102)


if you use stout's hashmap, you can do

`!fields.contains(key)`



src/linux/perf.hpp (lines 108 - 110)


why are these public?



src/linux/perf.hpp (line 114)


use stout's hashmap. it's an unordered map.



src/linux/perf.hpp (line 163)


const std::string& ?



src/linux/perf.hpp (line 165)


const Duration& ?



src/linux/perf.hpp (line 196)


s/subSys/subsystem/



src/linux/perf.hpp (line 199)


s/,/;/
s/TraceEvent's/TraceEvents/



src/linux/perf.hpp (lines 206 - 207)


s/,/;/
s/TraceEvent's/TraceEvents/



src/linux/perf.hpp (line 213)


kill extra new line.



src/linux/perf.hpp (lines 216 - 217)


don't think you need this.



src/linux/perf.hpp (lines 229 - 231)


these should be const?



src/linux/perf.cpp (line 575)


const string&



src/linux/perf.cpp (line 577)


const Duration&



src/linux/perf.cpp (line 590)


const string&



src/linux/perf.cpp (lines 920 - 931)


pull this above schedEvents to #910.



src/linux/perf.cpp (line 939)


hmm. what happens if another `parse` call comes in while the previous one 
is in progress?



src/linux/perf.cpp (line 951)


what if the user discards this future?



src/linux/perf.cpp (line 974)


s/ehdr/header/

or

s/ehdr/eventHeader/



src/linux/perf.cpp (lines 974 - 1029)


this could use some comments.



src/linux/perf.cpp (line 1056)


const string&



src/linux/perf.cpp (lines 1085 - 1086)


indent by 2 spaces.



src/linux/perf.cpp (line 1098)


s/subSys/subsystem/



src/linux/perf.cpp (lines 1125 - 1128)


this could use some comments.



src/linux/perf.cpp (lines 1144 - 1164)


can you add some new lines to separate blocks here. it is too dense.



src/linux/perf.cpp (lines 1209 - 1213)


why do you need to manually do this?



src/tests/containerizer/cgroups_tests.cpp (line 1093)


kill this.



src/tests/containerizer/cgroups_tests.cpp (lines 1101 - 1102)


`EXPECT_SOME(event.get("common_pid"));` ?



src/tests/containerizer/cgroups_tests.cpp (line 1105)


no need for this?



src/tests/containerizer/perf_tests.cpp (lines 205 - 206)


ditto.


- Vinod Kone


On Sept. 30, 2015, 12:14 a.m., Cong Wang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37541/
> ---
> 
> (Updated Sept. 30, 2015, 12:14 a.m.)
> 
> 
> Review request for mesos, Chi Zhang, Ian Downes, Jie Yu, and Vinod Kone.
> 
> 
> Bugs: MESOS-2769
> https://issues.apache.org/jira/browse/MESOS-2769
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Based on the PerfEvent API's, add API for Linux kernel trace events, 
> especially the schedule trace events.
> 
> 
> Diffs
> -
> 
>   src/linux/perf.hpp d10968ca670eb516bae08385fd0ddde8e8ad83b5 
>   src/linux/perf.cpp f7035ddb2507a7646d88dd517d048018f695448a 
>   

Re: Review Request 37541: Add trace event API

[Cong Wang]

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

(Updated Sept. 30, 2015, 12:14 a.m.)


Review request for mesos, Chi Zhang, Ian Downes, Jie Yu, and Vinod Kone.


Changes
---

Cleanup and rebase


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


Repository: mesos


Description
---

Based on the PerfEvent API's, add API for Linux kernel trace events, especially 
the schedule trace events.


Diffs (updated)
-

  src/linux/perf.hpp d10968ca670eb516bae08385fd0ddde8e8ad83b5 
  src/linux/perf.cpp f7035ddb2507a7646d88dd517d048018f695448a 
  src/tests/containerizer/cgroups_tests.cpp 
75a3bc0009c037dc18ce319db2eb44630f083e8c 
  src/tests/containerizer/perf_tests.cpp 
ed5212ee31b8aa47339b8b8fab184bbdf85be82a 

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


Testing
---

make check


Thanks,

Cong Wang

Re: Review Request 37541: Add trace event API

[Cong Wang]

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

(Updated Sept. 18, 2015, 10:12 p.m.)


Review request for mesos, Chi Zhang, Ian Downes, Jie Yu, and Vinod Kone.


Changes
---

Rebase and cleanup


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


Repository: mesos


Description
---

Based on the PerfEvent API's, add API for Linux kernel trace events, especially 
the schedule trace events.


Diffs (updated)
-

  src/linux/perf.hpp d10968ca670eb516bae08385fd0ddde8e8ad83b5 
  src/linux/perf.cpp f7035ddb2507a7646d88dd517d048018f695448a 
  src/tests/containerizer/cgroups_tests.cpp 
75a3bc0009c037dc18ce319db2eb44630f083e8c 
  src/tests/containerizer/perf_tests.cpp 
ed5212ee31b8aa47339b8b8fab184bbdf85be82a 

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


Testing
---

make check


Thanks,

Cong Wang

Re: Review Request 37541: Add trace event API

[Cong Wang]

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

(Updated Sept. 4, 2015, 11:15 p.m.)


Review request for mesos, Chi Zhang, Ian Downes, Jie Yu, and Vinod Kone.


Changes
---

Rebase and cleanup


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


Repository: mesos


Description
---

Based on the PerfEvent API's, add API for Linux kernel trace events, especially 
the schedule trace events.


Diffs (updated)
-

  src/linux/perf.hpp d10968ca670eb516bae08385fd0ddde8e8ad83b5 
  src/linux/perf.cpp 0011482cf9d920485728798518d32af0e9627724 
  src/tests/containerizer/cgroups_tests.cpp 
75a3bc0009c037dc18ce319db2eb44630f083e8c 
  src/tests/containerizer/perf_tests.cpp 
bef475e4b573f02b69f2c4dd6d1c6a6e10f34c38 

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


Testing
---

make check


Thanks,

Cong Wang

Re: Review Request 37541: Add trace event API

[Cong Wang]

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

(Updated Sept. 2, 2015, 10:17 p.m.)


Review request for mesos, Chi Zhang, Ian Downes, Jie Yu, and Vinod Kone.


Summary (updated)
-

Add trace event API


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


Repository: mesos


Description
---

Based on the PerfEvent API's, add API for Linux kernel trace events, especially 
the schedule trace events.


Diffs
-

  src/linux/perf.hpp c5630118f4858b1a805f25a2db7a24ca0989 
  src/linux/perf.cpp bb8b5918e4c8f4379784d4fa18c3b4709696e1ea 
  src/tests/containerizer/cgroups_tests.cpp 
75a3bc0009c037dc18ce319db2eb44630f083e8c 
  src/tests/containerizer/perf_tests.cpp 
8c29ca3dc2a8f6d1fb0f430482190f87f08eb4e8 

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


Testing
---

make check


Thanks,

Cong Wang