Repository: mesos
Updated Branches:
  refs/heads/master 1d86c5ca1 -> dbc41f2ff


Fixed perf stat output parsing.

This change adds a workaround for a bug in `perf stat`
(https://lkml.org/lkml/2018/3/6/22) and fixes handling
of nameless counters.

Review: https://reviews.apache.org/r/66030/


Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/dbc41f2f
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/dbc41f2f
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/dbc41f2f

Branch: refs/heads/master
Commit: dbc41f2ffb925cecc4b1096811a94c3bea0003e6
Parents: 1d86c5c
Author: Ilya Pronin <ipro...@twopensource.com>
Authored: Tue Mar 13 15:46:48 2018 -0700
Committer: James Peach <jpe...@apache.org>
Committed: Tue Mar 13 16:15:52 2018 -0700

----------------------------------------------------------------------
 src/linux/perf.cpp                     | 22 ++++++++++++++++++++++
 src/tests/containerizer/perf_tests.cpp | 25 +++++++++++++++++++++++++
 2 files changed, 47 insertions(+)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/dbc41f2f/src/linux/perf.cpp
----------------------------------------------------------------------
diff --git a/src/linux/perf.cpp b/src/linux/perf.cpp
index b301e25..f5ac21d 100644
--- a/src/linux/perf.cpp
+++ b/src/linux/perf.cpp
@@ -377,6 +377,22 @@ struct Sample
       case 8:
         return Sample({tokens[0], internal::normalize(tokens[2]), tokens[3]});
 
+      // This is the same format as the one with 8 fields. Due to a
+      // bug 'perf stat' may print 4 CSV separators instead of 2 empty
+      // fields (https://lkml.org/lkml/2018/3/6/22).
+      case 10:
+        // Check that the last 4 fields are empty. Otherwise this is
+        // an unknown format.
+        for (int i = 6; i < 10; ++i) {
+          if (!tokens[i].empty()) {
+            return Error(
+                "Unexpected number of fields (" + stringify(tokens.size()) +
+                ")");
+          }
+        }
+
+        return Sample({tokens[0], internal::normalize(tokens[2]), tokens[3]});
+
       // Bail out if the format is not recognized.
       default:
         return Error(
@@ -399,6 +415,12 @@ Try<hashmap<string, mesos::PerfStatistics>> parse(
                    sample.error());
     }
 
+    // Some additional metrics (e.g. stalled cycles per instruction)
+    // are printed without an event. Ignore them.
+    if (sample->event.empty()) {
+      continue;
+    }
+
     if (!statistics.contains(sample->cgroup)) {
       statistics.put(sample->cgroup, mesos::PerfStatistics());
     }

http://git-wip-us.apache.org/repos/asf/mesos/blob/dbc41f2f/src/tests/containerizer/perf_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/containerizer/perf_tests.cpp 
b/src/tests/containerizer/perf_tests.cpp
index 7176240..915ad18 100644
--- a/src/tests/containerizer/perf_tests.cpp
+++ b/src/tests/containerizer/perf_tests.cpp
@@ -120,6 +120,31 @@ TEST_F(PerfTest, Parse)
   EXPECT_TRUE(statistics.has_task_clock());
   EXPECT_EQ(0.0, statistics.task_clock());
 
+  // Due to a bug 'perf stat' may append extra CSV separators. Check
+  // that we handle this situation correctly.
+  parse = perf::parse(
+      "<not supported>,,stalled-cycles-backend,cgroup1,0,100.00,,,,");
+  ASSERT_SOME(parse);
+  ASSERT_TRUE(parse->contains("cgroup1"));
+
+  statistics = parse->get("cgroup1").get();
+  EXPECT_FALSE(statistics.has_stalled_cycles_backend());
+
+  // Some additional metrics (e.g. stalled cycles per instruction) can
+  // be printed without an event. They should be ignored.
+  parse = perf::parse(
+      "11651954,,instructions,cgroup1,1041690,10.63,0.58,insn per cycle\n"
+      ",,,,,,1.29,stalled cycles per insn\n"
+      "14995512,,stalled-cycles-frontend,cgroup1,1464204,14.94,75.26,frontend 
cycles idle"); // NOLINT(whitespace/line_length)
+  ASSERT_SOME(parse);
+  ASSERT_TRUE(parse->contains("cgroup1"));
+
+  statistics = parse->get("cgroup1").get();
+  EXPECT_TRUE(statistics.has_instructions());
+  EXPECT_EQ(11651954u, statistics.instructions());
+  EXPECT_TRUE(statistics.has_stalled_cycles_frontend());
+  EXPECT_EQ(14995512u, statistics.stalled_cycles_frontend());
+
   // Check parsing fails.
   parse = perf::parse("1,cycles\ngarbage");
   EXPECT_ERROR(parse);

Reply via email to