Merge "simpleperf: fix more recording time than requested."android-o-mr1-iot-release-1.0.2

6 files changed, 73 insertions, 17 deletions

diff --git a/simpleperf/RecordReadThread.cpp b/simpleperf/RecordReadThread.cpp
index d0e0735c..531da988 100644
--- a/simpleperf/RecordReadThread.cpp
+++ b/simpleperf/RecordReadThread.cpp
@@ -216,8 +216,7 @@ RecordReadThread::RecordReadThread(size_t record_buffer_size, const perf_event_a
 
 RecordReadThread::~RecordReadThread() {
   if (read_thread_) {
-    SendCmdToReadThread(CMD_STOP_THREAD, nullptr);
-    read_thread_->join();
+    StopReadThread();
   }
 }
 
@@ -235,13 +234,7 @@ bool RecordReadThread::RegisterDataCallback(IOEventLoop& loop,
   read_data_fd_.reset(data_fd[0]);
   write_data_fd_.reset(data_fd[1]);
   has_data_notification_ = false;
-  auto callback = [this, data_callback]() {
-    char dummy;
-    TEMP_FAILURE_RETRY(read(read_data_fd_, &dummy, 1));
-    has_data_notification_ = false;
-    return data_callback();
-  };
-  if (!loop.AddReadEvent(read_data_fd_, callback)) {
+  if (!loop.AddReadEvent(read_data_fd_, data_callback)) {
     return false;
   }
   read_thread_.reset(new std::thread([&]() { RunReadThread(); }));
@@ -260,6 +253,15 @@ bool RecordReadThread::SyncKernelBuffer() {
   return SendCmdToReadThread(CMD_SYNC_KERNEL_BUFFER, nullptr);
 }
 
+bool RecordReadThread::StopReadThread() {
+  bool result = SendCmdToReadThread(CMD_STOP_THREAD, nullptr);
+  if (result) {
+    read_thread_->join();
+    read_thread_ = nullptr;
+  }
+  return result;
+}
+
 bool RecordReadThread::SendCmdToReadThread(Cmd cmd, void* cmd_arg) {
   {
     std::lock_guard<std::mutex> lock(cmd_mutex_);
@@ -280,7 +282,15 @@ bool RecordReadThread::SendCmdToReadThread(Cmd cmd, void* cmd_arg) {
 std::unique_ptr<Record> RecordReadThread::GetRecord() {
   record_buffer_.MoveToNextRecord();
   char* p = record_buffer_.GetCurrentRecord();
-  return p != nullptr ? ReadRecordFromBuffer(attr_, p) : nullptr;
+  if (p != nullptr) {
+    return ReadRecordFromBuffer(attr_, p);
+  }
+  if (has_data_notification_) {
+    char dummy;
+    TEMP_FAILURE_RETRY(read(read_data_fd_, &dummy, 1));
+    has_data_notification_ = false;
+  }
+  return nullptr;
 }
 
 void RecordReadThread::RunReadThread() {
diff --git a/simpleperf/RecordReadThread.h b/simpleperf/RecordReadThread.h
index e4088fb4..c4a6830a 100644
--- a/simpleperf/RecordReadThread.h
+++ b/simpleperf/RecordReadThread.h
@@ -132,6 +132,8 @@ class RecordReadThread {
   bool RemoveEventFds(const std::vector<EventFd*>& event_fds);
   // Move all available records in kernel buffers to the RecordBuffer.
   bool SyncKernelBuffer();
+  // Stop the read thread, no more records will be put into the RecordBuffer.
+  bool StopReadThread();
 
   // If available, return the next record in the RecordBuffer, otherwise return nullptr.
   std::unique_ptr<Record> GetRecord();
diff --git a/simpleperf/RecordReadThread_test.cpp b/simpleperf/RecordReadThread_test.cpp
index 8c34ffd8..a39b30ac 100644
--- a/simpleperf/RecordReadThread_test.cpp
+++ b/simpleperf/RecordReadThread_test.cpp
@@ -247,6 +247,7 @@ TEST_F(RecordReadThreadTest, handle_cmds) {
   ASSERT_TRUE(has_notify);
   ASSERT_TRUE(thread.GetRecord());
   ASSERT_TRUE(thread.RemoveEventFds(event_fds));
+  ASSERT_TRUE(thread.StopReadThread());
 }
 
 TEST_F(RecordReadThreadTest, read_records) {
@@ -336,3 +337,31 @@ TEST_F(RecordReadThreadTest, process_sample_record) {
   ASSERT_EQ(lost_non_samples, 0u);
   ASSERT_EQ(cut_stack_samples, 1u);
 }
+
+// Test that the data notification exists until the RecordBuffer is empty. So we can read all
+// records even if reading one record at a time.
+TEST_F(RecordReadThreadTest, has_data_notification_until_buffer_empty) {
+  perf_event_attr attr = CreateFakeEventAttr();
+  RecordReadThread thread(128 * 1024, attr, 1, 1);
+  IOEventLoop loop;
+  size_t record_index = 0;
+  auto read_one_record = [&]() {
+    std::unique_ptr<Record> r = thread.GetRecord();
+    if (!r) {
+      return loop.ExitLoop();
+    }
+    std::unique_ptr<Record>& expected = records_[record_index++];
+    if (r->size() != expected->size() || memcmp(r->Binary(), expected->Binary(), r->size()) != 0) {
+      return false;
+    }
+    return true;
+  };
+  ASSERT_TRUE(thread.RegisterDataCallback(loop, read_one_record));
+  records_ = CreateFakeRecords(attr, 2, 0, 0);
+  std::vector<EventFd*> event_fds = CreateFakeEventFds(attr, 1);
+  ASSERT_TRUE(thread.AddEventFds(event_fds));
+  ASSERT_TRUE(thread.SyncKernelBuffer());
+  ASSERT_TRUE(loop.RunLoop());
+  ASSERT_EQ(record_index, records_.size());
+  ASSERT_TRUE(thread.RemoveEventFds(event_fds));
+}
diff --git a/simpleperf/cmd_record.cpp b/simpleperf/cmd_record.cpp
index 6a069cd5..e58e6594 100644
--- a/simpleperf/cmd_record.cpp
+++ b/simpleperf/cmd_record.cpp
@@ -1190,7 +1190,7 @@ bool RecordCommand::ProcessJITDebugInfo(const std::vector<JITSymFile>& jit_symfi
   // generated after them. So process existing samples each time generating new JIT maps. We prefer
   // to process samples after processing JIT maps. Because some of the samples may hit the new JIT
   // maps, and we want to report them properly.
-  if (sync_kernel_records && !event_selection_set_.ReadMmapEventData(true)) {
+  if (sync_kernel_records && !event_selection_set_.SyncKernelBuffer()) {
     return false;
   }
   return true;
diff --git a/simpleperf/event_selection_set.cpp b/simpleperf/event_selection_set.cpp
index a69f6173..5990da4b 100644
--- a/simpleperf/event_selection_set.cpp
+++ b/simpleperf/event_selection_set.cpp
@@ -595,7 +595,7 @@ bool EventSelectionSet::PrepareToReadMmapEventData(const std::function<bool(Reco
   // Prepare record callback function.
   record_callback_ = callback;
   if (!record_read_thread_->RegisterDataCallback(*loop_,
-                                                 [this]() { return ReadMmapEventData(false); })) {
+                                                 [this]() { return ReadMmapEventData(true); })) {
     return false;
   }
   std::vector<EventFd*> event_fds;
@@ -609,21 +609,35 @@ bool EventSelectionSet::PrepareToReadMmapEventData(const std::function<bool(Reco
   return record_read_thread_->AddEventFds(event_fds);
 }
 
-bool EventSelectionSet::ReadMmapEventData(bool sync_kernel_buffer) {
-  if (sync_kernel_buffer && !record_read_thread_->SyncKernelBuffer()) {
-    return false;
+bool EventSelectionSet::SyncKernelBuffer() {
+  return record_read_thread_->SyncKernelBuffer();
+}
+
+// Read records from the RecordBuffer. If with_time_limit is false, read until the RecordBuffer is
+// empty, otherwise stop after 100 ms or when the record buffer is empty.
+bool EventSelectionSet::ReadMmapEventData(bool with_time_limit) {
+  uint64_t start_time_in_ns;
+  if (with_time_limit) {
+    start_time_in_ns = GetSystemClock();
   }
   std::unique_ptr<Record> r;
   while ((r = record_read_thread_->GetRecord()) != nullptr) {
     if (!record_callback_(r.get())) {
       return false;
     }
+    if (with_time_limit && (GetSystemClock() - start_time_in_ns) >= 1e8) {
+      break;
+    }
   }
   return true;
 }
 
 bool EventSelectionSet::FinishReadMmapEventData() {
-  if (!ReadMmapEventData(true)) {
+  // Stop the read thread, so we don't get more records beyond current time.
+  if (!SyncKernelBuffer() || !record_read_thread_->StopReadThread()) {
+    return false;
+  }
+  if (!ReadMmapEventData(false)) {
     return false;
   }
   if (!HasInplaceSampler()) {
diff --git a/simpleperf/event_selection_set.h b/simpleperf/event_selection_set.h
index e5d77e75..f8067868 100644
--- a/simpleperf/event_selection_set.h
+++ b/simpleperf/event_selection_set.h
@@ -134,7 +134,7 @@ class EventSelectionSet {
   bool ReadCounters(std::vector<CountersInfo>* counters);
   bool MmapEventFiles(size_t min_mmap_pages, size_t max_mmap_pages, size_t record_buffer_size);
   bool PrepareToReadMmapEventData(const std::function<bool(Record*)>& callback);
-  bool ReadMmapEventData(bool sync_kernel_buffer);
+  bool SyncKernelBuffer();
   bool FinishReadMmapEventData();
   void GetLostRecords(size_t* lost_samples, size_t* lost_non_samples, size_t* cut_stack_samples);
 
@@ -166,6 +166,7 @@ class EventSelectionSet {
                                     const std::map<pid_t, std::set<pid_t>>& process_map);
   bool OpenEventFilesOnGroup(EventSelectionGroup& group, pid_t tid, int cpu,
                              std::string* failed_event_type);
+  bool ReadMmapEventData(bool with_time_limit);
 
   bool DetectCpuHotplugEvents();
   bool HandleCpuOnlineEvent(int cpu);