----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37417/#review95777 -----------------------------------------------------------
Paul can you split this change? I personally don't have context on why we would want to change from sampling 'true' to running against the init process so I'd like Jie or Ian to review that change, but I can help you make the refactorings to use 'Perf'. Sound good? src/linux/perf.cpp (lines 395 - 408) <https://reviews.apache.org/r/37417/#comment150891> Looks like change is doing more than just using the Perf object. You've also changed this to work off of init rather than the 'true' command? Why the latter change? (Not clear from the review description or ticket) Also, any reason you introduce internal::validate()? Looks like it can just be inlined inside valid()? src/linux/perf.cpp (lines 464 - 467) <https://reviews.apache.org/r/37417/#comment150892> Let's try to avoid jaggedness in comments :) src/linux/perf.cpp (lines 473 - 474) <https://reviews.apache.org/r/37417/#comment150894> future.await() is an anti-pattern, so let's explain why it's needed here. I assume it's because isolator creation is synchronous currently and making it asynchronous is a larger undertaking? src/linux/perf.cpp (line 476) <https://reviews.apache.org/r/37417/#comment150893> Please remember to put spaces between your if and open paren. Ideally this should be automated, but it's not right now :) - Ben Mahler On Aug. 18, 2015, 6:43 p.m., Paul Brett wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/37417/ > ----------------------------------------------------------- > > (Updated Aug. 18, 2015, 6:43 p.m.) > > > Review request for mesos and Ben Mahler. > > > Bugs: MESOS-3185 > https://issues.apache.org/jira/browse/MESOS-3185 > > > Repository: mesos > > > Description > ------- > > Convert Perf event validator to use new shared object. > > > Diffs > ----- > > src/linux/perf.cpp cb1a13d0b1754a50f0121bfda522056ff8c3e3c8 > > Diff: https://reviews.apache.org/r/37417/diff/ > > > Testing > ------- > > sudo make check > > > Thanks, > > Paul Brett > >
