From 825f098bc440fb8dcdfce70b895eb0f7faa71f4f Mon Sep 17 00:00:00 2001
From: Robert O'Callahan <robert@ocallahan.org>
Date: Sat, 16 Dec 2023 16:30:09 +1300
Subject: [PATCH] Fold PerfCounters::read_ticks() API into stop() to simplify
 things

---
 src/PerfCounters.cc | 18 +++++++++++-------
 src/PerfCounters.h  | 14 ++++++--------
 src/Task.cc         | 11 ++++-------
 3 files changed, 21 insertions(+), 22 deletions(-)

diff --git a/src/PerfCounters.cc b/src/PerfCounters.cc
index 70c5a7091f2..8f0ae9e355d 100644
--- a/src/PerfCounters.cc
+++ b/src/PerfCounters.cc
@@ -778,11 +778,14 @@ void PerfCounters::PTState::close() {
   }
 }
 
-void PerfCounters::start(Ticks ticks_period) {
+void PerfCounters::start(Task* t, Ticks ticks_period) {
+  ASSERT(t, !counting);
+  ASSERT(t, ticks_period >= 0);
+
   if (enabled == DISABLE) {
     return;
   }
-  DEBUG_ASSERT(ticks_period >= 0);
+
   check_pmu(pmu_index);
 
   auto &perf_attr = perf_attrs[pmu_index];
@@ -874,10 +877,11 @@ void PerfCounters::close() {
   fd_ticks_in_transaction.close();
 }
 
-void PerfCounters::stop() {
+Ticks PerfCounters::stop(Task* t) {
   if (!counting) {
-    return;
+    return 0;
   }
+  Ticks ticks = read_ticks(t);
   counting = false;
   if (always_recreate_counters(perf_attrs[pmu_index])) {
     close();
@@ -890,6 +894,7 @@ void PerfCounters::stop() {
       infallible_perf_event_disable_if_open(pt_state->pt_perf_event_fd);
     }
   }
+  return ticks;
 }
 
 // Note that on aarch64 this is also used to get the count for `ret`
@@ -909,9 +914,8 @@ Ticks PerfCounters::ticks_for_direct_call(Task*) {
 }
 
 Ticks PerfCounters::read_ticks(Task* t) {
-  if (!opened || !counting) {
-    return 0;
-  }
+  ASSERT(t, opened);
+  ASSERT(t, counting);
 
   if (pt_state) {
     pt_state->flush();
diff --git a/src/PerfCounters.h b/src/PerfCounters.h
index 3e257f8dafe..15e03a83162 100644
--- a/src/PerfCounters.h
+++ b/src/PerfCounters.h
@@ -91,12 +91,14 @@ class PerfCounters {
    * `ticks_period` of zero means don't interrupt at all.
    * Opens all relevant fds if necessary.
    */
-  void start(Ticks ticks_period);
+  void start(Task* t, Ticks ticks_period);
 
   /**
    * Suspend counting until the next start.
+   * Returns the current value of the ticks counter.
+   * `t` is used for debugging purposes.
    */
-  void stop();
+  Ticks stop(Task* t);
 
   /**
    * Close the perfcounter fds (if open). They will be automatically reopened if/when
@@ -122,12 +124,6 @@ class PerfCounters {
    */
   static bool support_cpu(int cpu);
 
-  /**
-   * Read the current value of the ticks counter.
-   * `t` is used for debugging purposes.
-   */
-  Ticks read_ticks(Task* t);
-
   /**
    * Returns what ticks mean for these counters.
    */
@@ -171,6 +167,8 @@ class PerfCounters {
    */
   uint32_t recording_skid_size() { return skid_size() * 5; }
 
+  Ticks read_ticks(Task* t);
+
   // Only valid while 'counting' is true
   Ticks counting_period;
   pid_t tid;
diff --git a/src/Task.cc b/src/Task.cc
index 33dacd44560..cc5361d5dde 100644
--- a/src/Task.cc
+++ b/src/Task.cc
@@ -1467,10 +1467,10 @@ bool Task::resume_execution(ResumeRequest how, WaitRequest wait_how,
 
   if (tick_period != RESUME_NO_TICKS) {
     if (tick_period == RESUME_UNLIMITED_TICKS) {
-      hpc.start(0);
+      hpc.start(this, 0);
     } else {
       ASSERT(this, tick_period >= 0 && tick_period <= MAX_TICKS_REQUEST);
-      hpc.start(max<Ticks>(1, tick_period));
+      hpc.start(this, max<Ticks>(1, tick_period));
     }
     activate_preload_thread_locals();
   }
@@ -2224,14 +2224,14 @@ bool Task::did_waitpid(WaitStatus status) {
         // For example if the task is already reaped we don't have new
         // register values and we don't want to read a ticks value
         // that mismatches our registers.
-        more_ticks = hpc.read_ticks(this);
+        more_ticks = hpc.stop(this);
       }
 #elif defined(__aarch64__)
       struct iovec vec = { &ptrace_regs,
                           sizeof(ptrace_regs) };
       if (ptrace_if_stopped(PTRACE_GETREGSET, NT_PRSTATUS, &vec)) {
         registers.set_from_ptrace(ptrace_regs);
-        more_ticks = hpc.read_ticks(this);
+        more_ticks = hpc.stop(this);
       }
 #else
 #error detect architecture here
@@ -2253,9 +2253,6 @@ bool Task::did_waitpid(WaitStatus status) {
   // Some (all?) SIGTRAP stops are *not* usable for signal injection.
   in_injectable_signal_stop_ =
     status.stop_sig() > 0 && status.stop_sig() != SIGTRAP;
-  // We stop counting here because there may be things we want to do to the
-  // tracee that would otherwise generate ticks.
-  hpc.stop();
   session().accumulate_ticks_processed(more_ticks);
   ticks += more_ticks;