Repository: mesos Updated Branches: refs/heads/master 65cc98879 -> f98bd5f5c
Remove unnecessary perf version checks. We can remove the version check from the perf subprocess wrapper because we longer uses the version to figure out how to parse the stat output and the callers explicitly check perf::supported(). Review: https://reviews.apache.org/r/56732/ Project: http://git-wip-us.apache.org/repos/asf/mesos/repo Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/f98bd5f5 Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/f98bd5f5 Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/f98bd5f5 Branch: refs/heads/master Commit: f98bd5f5cb8c877f7f96e5d1529e79dd526bb8e1 Parents: 65cc988 Author: James Peach <[email protected]> Authored: Fri Apr 28 15:34:27 2017 -0700 Committer: Jiang Yan Xu <[email protected]> Committed: Fri Apr 28 16:17:30 2017 -0700 ---------------------------------------------------------------------- src/linux/perf.cpp | 48 +++++++++++------------------ src/linux/perf.hpp | 3 +- src/tests/containerizer/perf_tests.cpp | 12 +++----- 3 files changed, 24 insertions(+), 39 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/mesos/blob/f98bd5f5/src/linux/perf.cpp ---------------------------------------------------------------------- diff --git a/src/linux/perf.cpp b/src/linux/perf.cpp index b333496..b301e25 100644 --- a/src/linux/perf.cpp +++ b/src/linux/perf.cpp @@ -300,35 +300,24 @@ Future<hashmap<string, mesos::PerfStatistics>> sample( Future<string> output = perf->output(); spawn(perf, true); - auto parse = [start, duration]( - const tuple<Version, string> values) -> - Future<hashmap<string, mesos::PerfStatistics>> { - const Version& version = std::get<0>(values); - const string& output = std::get<1>(values); - - // Check that the version is supported. - if (!supported(version)) { - return Failure("Perf " + stringify(version) + " is not supported"); - } - - Try<hashmap<string, mesos::PerfStatistics>> result = - perf::parse(output, version); - - if (result.isError()) { - return Failure("Failed to parse perf sample: " + result.error()); - } + // TODO(pbrett): Don't wait for these forever! + return output + .then([start, duration](const string output) + -> Future<hashmap<string, mesos::PerfStatistics>> { + Try<hashmap<string, mesos::PerfStatistics>> result = + perf::parse(output); - foreachvalue (mesos::PerfStatistics& statistics, result.get()) { - statistics.set_timestamp(start.secs()); - statistics.set_duration(duration.secs()); - } + if (result.isError()) { + return Failure("Failed to parse perf sample: " + result.error()); + } - return result.get(); - }; + foreachvalue (mesos::PerfStatistics& statistics, result.get()) { + statistics.set_timestamp(start.secs()); + statistics.set_duration(duration.secs()); + } - // TODO(pbrett): Don't wait for these forever! - return process::collect(perf::version(), output) - .then(parse); + return result.get(); + }); } @@ -362,7 +351,7 @@ struct Sample // Convert a single line of perf output in CSV format (using // PERF_DELIMITER as a separator) to a sample. - static Try<Sample> parse(const string& line, const Version& version) + static Try<Sample> parse(const string& line) { // We use strings::split to separate the tokens // because the unit field can be empty. @@ -398,13 +387,12 @@ struct Sample Try<hashmap<string, mesos::PerfStatistics>> parse( - const string& output, - const Version& version) + const string& output) { hashmap<string, mesos::PerfStatistics> statistics; foreach (const string& line, strings::tokenize(output, "\n")) { - Try<Sample> sample = Sample::parse(line, version); + Try<Sample> sample = Sample::parse(line); if (sample.isError()) { return Error("Failed to parse perf sample line '" + line + "': " + http://git-wip-us.apache.org/repos/asf/mesos/blob/f98bd5f5/src/linux/perf.hpp ---------------------------------------------------------------------- diff --git a/src/linux/perf.hpp b/src/linux/perf.hpp index 20ebcf1..7c4a725 100644 --- a/src/linux/perf.hpp +++ b/src/linux/perf.hpp @@ -63,8 +63,7 @@ Try<Version> parseVersion(const std::string& output); // Note: The parse function is exposed to allow testing of the // multiple supported perf stat output formats. Try<hashmap<std::string, mesos::PerfStatistics>> parse( - const std::string& output, - const Version& version); + const std::string& output); } // namespace perf { http://git-wip-us.apache.org/repos/asf/mesos/blob/f98bd5f5/src/tests/containerizer/perf_tests.cpp ---------------------------------------------------------------------- diff --git a/src/tests/containerizer/perf_tests.cpp b/src/tests/containerizer/perf_tests.cpp index b33b928..d8aab08 100644 --- a/src/tests/containerizer/perf_tests.cpp +++ b/src/tests/containerizer/perf_tests.cpp @@ -77,8 +77,7 @@ TEST_F(PerfTest, Parse) "0.456,task-clock,cgroup2\n" "0.123,task-clock,cgroup1\n" "5812843447,,cycles,cgroup3,3560494814,100.00,0.097,GHz\n" - "60011.034108,,task-clock,cgroup3,60011034108,100.00,11.999,CPUs utilized", // NOLINT(whitespace/line_length) - Version(3, 12, 0)); + "60011.034108,,task-clock,cgroup3,60011034108,100.00,11.999,CPUs utilized"); // NOLINT(whitespace/line_length) ASSERT_SOME(parse); EXPECT_EQ(3u, parse->size()); @@ -100,7 +99,7 @@ TEST_F(PerfTest, Parse) EXPECT_EQ(0.456, statistics.task_clock()); // Statistics reporting <not supported> should not appear. - parse = perf::parse("<not supported>,cycles,cgroup1", Version(3, 12, 0)); + parse = perf::parse("<not supported>,cycles,cgroup1"); ASSERT_SOME(parse); ASSERT_TRUE(parse->contains("cgroup1")); @@ -109,8 +108,7 @@ TEST_F(PerfTest, Parse) // Statistics reporting <not counted> should be zero. parse = perf::parse("<not counted>,cycles,cgroup1\n" - "<not counted>,task-clock,cgroup1", - Version(3, 12, 0)); + "<not counted>,task-clock,cgroup1"); ASSERT_SOME(parse); ASSERT_TRUE(parse->contains("cgroup1")); @@ -122,10 +120,10 @@ TEST_F(PerfTest, Parse) EXPECT_EQ(0.0, statistics.task_clock()); // Check parsing fails. - parse = perf::parse("1,cycles\ngarbage", Version(3, 12, 0)); + parse = perf::parse("1,cycles\ngarbage"); EXPECT_ERROR(parse); - parse = perf::parse("1,unknown-field", Version(3, 12, 0)); + parse = perf::parse("1,unknown-field"); EXPECT_ERROR(parse); }
