mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Cong Wang" <cw...@twopensource.com>
Subject Re: Review Request 37540: Add perf event API
Date Tue, 15 Sep 2015 22:33:57 GMT


> On Sept. 15, 2015, 12:31 a.m., Vinod Kone wrote:
> > my main question is regarding division of responsibilities between the wrapper class
and the process class. Looks like opening the fd and mmaping is done by the wrapper and the
rest by the process. Can everything be done by the process?

Hmm, but why should we expose the process class to user? With current code, user only sees
the wrapper. Also, there are many similar code in src/linux/cgroup.cpp, pretty much because
of the async IO interface in Mesos code base.


> On Sept. 15, 2015, 12:31 a.m., Vinod Kone wrote:
> > src/linux/perf.cpp, line 747
> > <https://reviews.apache.org/r/37540/diff/6/?file=1064305#file1064305line747>
> >
> >     is there any reason why we want close() as part of the public API? sounds like
users can simply call delete on the PerfEvent object? i would recommend removing this method
or making it private.
> >     
> >     more importantly, what happens if close() is called twice? is that safe?

Good point. Fixed, just FYI it is a smart pointer, user should call reset() not delete.


> On Sept. 15, 2015, 12:31 a.m., Vinod Kone wrote:
> > src/linux/perf.cpp, lines 749-752
> > <https://reviews.apache.org/r/37540/diff/6/?file=1064305#file1064305line749>
> >
> >     can this be done by the process instead?

Probably we can do it in finialize() too, not sure what is the benifit over this?


> On Sept. 15, 2015, 12:31 a.m., Vinod Kone wrote:
> > src/linux/perf.cpp, line 563
> > <https://reviews.apache.org/r/37540/diff/6/?file=1064305#file1064305line563>
> >
> >     sharing the map between this class and the constituent process class is unfortunate.
what happens if the process class deletes the pointer?
> >     
> >     is it possible to have the process class create and own the map?

The map is created when user calls create(), then it is passed to the process class, so the
process class only reads it when user calls read().


- Cong


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


On Sept. 4, 2015, 11:13 p.m., Cong Wang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37540/
> -----------------------------------------------------------
> 
> (Updated Sept. 4, 2015, 11:13 p.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
> -------
> 
> Abstract Linux kernel perf event API and provide API to collect schedule events.
> 
> 
> Diffs
> -----
> 
>   src/linux/perf.hpp d10968ca670eb516bae08385fd0ddde8e8ad83b5 
>   src/linux/perf.cpp 0011482cf9d920485728798518d32af0e9627724 
>   src/tests/containerizer/perf_tests.cpp bef475e4b573f02b69f2c4dd6d1c6a6e10f34c38 
> 
> Diff: https://reviews.apache.org/r/37540/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Cong Wang
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message