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);