Merge "simpleperf: notify user for unsupported modifiers."

am: 43908a31c1

Change-Id: I4f53fa84ea663a4090a99f2261f153faf8f0b8fb

diff --git a/simpleperf/cmd_record.cpp b/simpleperf/cmd_record.cpp
index 28b75c8..e746a9a 100644 (file)
--- a/simpleperf/cmd_record.cpp
+++ b/simpleperf/cmd_record.cpp
@@ -159,6 +159,7 @@ class RecordCommand : public Command {
         duration_in_sec_(0),
         can_dump_kernel_symbols_(true),
         dump_symbols_(false),
+        event_selection_set_(false),
         mmap_page_range_(std::make_pair(1, DESIRED_PAGES_IN_MAPPED_BUFFER)),
         record_filename_("perf.data"),
         start_sampling_time_in_ns_(0),

diff --git a/simpleperf/cmd_record_test.cpp b/simpleperf/cmd_record_test.cpp
index 0198fd1..9729904 100644 (file)
--- a/simpleperf/cmd_record_test.cpp
+++ b/simpleperf/cmd_record_test.cpp
@@ -314,3 +314,12 @@ TEST(record_cmd, duration_option) {
   ASSERT_TRUE(
       RecordCmd()->Run({"--duration", "1", "-o", tmpfile.path, "sleep", "2"}));
 }
+
+TEST(record_cmd, support_modifier_for_clock_events) {
+  for (const std::string& e : {"cpu-clock", "task-clock"}) {
+    for (const std::string& m : {"u", "k"}) {
+      ASSERT_TRUE(RunRecordCmd({"-e", e + ":" + m})) << "event " << e << ":"
+                                                     << m;
+    }
+  }
+}

diff --git a/simpleperf/cmd_stat.cpp b/simpleperf/cmd_stat.cpp
index 4199293..76df147 100644 (file)
--- a/simpleperf/cmd_stat.cpp
+++ b/simpleperf/cmd_stat.cpp
@@ -292,6 +292,7 @@ class StatCommand : public Command {
         system_wide_collection_(false),
         child_inherit_(true),
         duration_in_sec_(0),
+        event_selection_set_(true),
         csv_(false) {
     // Die if parent exits.
     prctl(PR_SET_PDEATHSIG, SIGHUP, 0, 0, 0);
@@ -380,7 +381,7 @@ bool StatCommand::Run(const std::vector<std::string>& args) {
   }
   if (duration_in_sec_ != 0) {
     if (!loop.AddPeriodicEvent(SecondToTimeval(duration_in_sec_),
-                           [&]() { return loop.ExitLoop(); })) {
+                               [&]() { return loop.ExitLoop(); })) {
       return false;
     }
   }

diff --git a/simpleperf/cmd_stat_test.cpp b/simpleperf/cmd_stat_test.cpp
index 1613021..5748b7a 100644 (file)
--- a/simpleperf/cmd_stat_test.cpp
+++ b/simpleperf/cmd_stat_test.cpp
@@ -86,24 +86,24 @@ TEST(stat_cmd, no_monitored_threads) { ASSERT_FALSE(StatCmd()->Run({""})); }
 TEST(stat_cmd, group_option) {
   ASSERT_TRUE(
       StatCmd()->Run({"--group", "cpu-cycles,cpu-clock", "sleep", "1"}));
-  ASSERT_TRUE(StatCmd()->Run({"--group", "cpu-cycles,cpu-clock", "--group",
-                              "cpu-cycles:u,cpu-clock:u", "--group",
-                              "cpu-cycles:k,cpu-clock:k", "sleep", "1"}));
+  ASSERT_TRUE(StatCmd()->Run({"--group", "cpu-cycles,instructions", "--group",
+                              "cpu-cycles:u,instructions:u", "--group",
+                              "cpu-cycles:k,instructions:k", "sleep", "1"}));
 }
 
 TEST(stat_cmd, auto_generated_summary) {
   TemporaryFile tmp_file;
-  ASSERT_TRUE(StatCmd()->Run({"--group", "cpu-clock:u,cpu-clock:k", "-o",
+  ASSERT_TRUE(StatCmd()->Run({"--group", "instructions:u,instructions:k", "-o",
                               tmp_file.path, "sleep", "1"}));
   std::string s;
   ASSERT_TRUE(android::base::ReadFileToString(tmp_file.path, &s));
-  size_t pos = s.find("cpu-clock:u");
+  size_t pos = s.find("instructions:u");
   ASSERT_NE(s.npos, pos);
-  pos = s.find("cpu-clock:k", pos);
+  pos = s.find("instructions:k", pos);
   ASSERT_NE(s.npos, pos);
-  pos += strlen("cpu-clock:k");
-  // Check if the summary of cpu-clock is generated.
-  ASSERT_NE(s.npos, s.find("cpu-clock", pos));
+  pos += strlen("instructions:k");
+  // Check if the summary of instructions is generated.
+  ASSERT_NE(s.npos, s.find("instructions", pos));
 }
 
 TEST(stat_cmd, duration_option) {
@@ -111,3 +111,12 @@ TEST(stat_cmd, duration_option) {
       StatCmd()->Run({"--duration", "1.2", "-p", std::to_string(getpid())}));
   ASSERT_TRUE(StatCmd()->Run({"--duration", "1", "sleep", "2"}));
 }
+
+TEST(stat_cmd, no_modifier_for_clock_events) {
+  for (const std::string& e : {"cpu-clock", "task-clock"}) {
+    for (const std::string& m : {"u", "k"}) {
+      ASSERT_FALSE(StatCmd()->Run({"-e", e + ":" + m, "sleep", "0.1"}))
+          << "event " << e << ":" << m;
+    }
+  }
+}

diff --git a/simpleperf/event_selection_set.cpp b/simpleperf/event_selection_set.cpp
index edff174..d6961e5 100644 (file)
--- a/simpleperf/event_selection_set.cpp
+++ b/simpleperf/event_selection_set.cpp
@@ -57,6 +57,17 @@ bool EventSelectionSet::BuildAndCheckEventSelection(
   if (event_type == nullptr) {
     return false;
   }
+  if (for_stat_cmd_) {
+    if (event_type->event_type.name == "cpu-clock" ||
+        event_type->event_type.name == "task-clock") {
+      if (event_type->exclude_user || event_type->exclude_kernel) {
+        LOG(ERROR) << "Modifier u and modifier k used in event type "
+                   << event_type->event_type.name
+                   << " are not supported by the kernel.";
+        return false;
+      }
+    }
+  }
   selection->event_type_modifier = *event_type;
   selection->event_attr = CreateDefaultPerfEventAttr(event_type->event_type);
   selection->event_attr.exclude_user = event_type->exclude_user;

diff --git a/simpleperf/event_selection_set.h b/simpleperf/event_selection_set.h
index c24dad4..6fb02b8 100644 (file)
--- a/simpleperf/event_selection_set.h
+++ b/simpleperf/event_selection_set.h
@@ -70,7 +70,7 @@ class IOEventLoop;
 
 class EventSelectionSet {
  public:
-  EventSelectionSet() {}
+  EventSelectionSet(bool for_stat_cmd) : for_stat_cmd_(for_stat_cmd) {}
 
   bool empty() const { return groups_.empty(); }
 
@@ -116,6 +116,8 @@ class EventSelectionSet {
 
   bool DetectCpuHotplugEvents();
 
+  const bool for_stat_cmd_;
+
   std::vector<EventSelectionGroup> groups_;
 
   std::function<bool(Record*)> record_callback_;