You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by ya...@apache.org on 2017/04/28 23:17:34 UTC
mesos git commit: Remove unnecessary perf version checks.
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 <jp...@apache.org>
Authored: Fri Apr 28 15:34:27 2017 -0700
Committer: Jiang Yan Xu <xu...@apple.com>
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);
}