> 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 > >
