Re: Review Request 37417: Convert Perf event validator to use new shared object.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37417/ --- (Updated Aug. 18, 2015, 6:42 p.m.) Review request for mesos and Ben Mahler. 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
Re: Review Request 37417: Convert Perf event validator to use new shared object.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37417/ --- (Updated Aug. 18, 2015, 6:12 p.m.) Review request for mesos and Ben Mahler. 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
Re: Review Request 37417: Convert Perf event validator to use new shared object.
--- 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
Re: Review Request 37417: Convert Perf event validator to use new shared object.
--- 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
Re: Review Request 37417: Convert Perf event validator to use new shared object.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37417/#review95806 --- Did you still want to use 'Perf' here? If not, I can't tell what we're gaining in this change, does supported() provide additional validation? - Ben Mahler On Aug. 18, 2015, 11:01 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, 11:01 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 cdc5f8314a875ababf2b17a32873775d808e1c78 Diff: https://reviews.apache.org/r/37417/diff/ Testing --- sudo make check Thanks, Paul Brett
Re: Review Request 37417: Convert Perf event validator to use new shared object.
On Aug. 18, 2015, 8:38 p.m., Ben Mahler wrote: 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? I will back out that change. - Paul --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37417/#review95777 --- On Aug. 18, 2015, 11:01 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, 11:01 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 cdc5f8314a875ababf2b17a32873775d808e1c78 Diff: https://reviews.apache.org/r/37417/diff/ Testing --- sudo make check Thanks, Paul Brett
Re: Review Request 37417: Convert Perf event validator to use new shared object.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37417/ --- (Updated Aug. 18, 2015, 11:01 p.m.) Review request for mesos and Ben Mahler. Changes --- Reduce scope of change per review. Bugs: MESOS-3185 https://issues.apache.org/jira/browse/MESOS-3185 Repository: mesos Description --- Convert Perf event validator to use new shared object. Diffs (updated) - src/linux/perf.cpp cdc5f8314a875ababf2b17a32873775d808e1c78 Diff: https://reviews.apache.org/r/37417/diff/ Testing --- sudo make check Thanks, Paul Brett
Review Request 37417: Convert Perf event validator to use new shared object.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37417/ --- Review request for mesos. 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
