commit 935ee775421e0c8a14f26669d9ab99f110ceb7d5
author Pavlin Radoslavov <pavlin@google.com> Mon Jan 08 11:37:05 2018 -0800
committer android-build-team Robot <android-build-team-robot@google.com> Thu Jan 18 19:07:55 2018 +0000
tree cae6dfda460b2d31fabd298ac2b65c9bbc8b2738
parent ff6208c4121dd6f14da465e3f5f4e0c30cd25d62

Removed alarm callback execution statistics

Updating the alarm state after the callback returns can be problematic
in case the callback itself deleted the alarm.

Bug: 67110137
Test: Manual
Change-Id: Id4de06eebedb792cadd63d09efb68672e9bddc69
Merged-In: Id4de06eebedb792cadd63d09efb68672e9bddc69
(cherry picked from commit 04574e1cde3b0d46b59b4b6ebab935ac60af9f97)
(cherry picked from commit 90ffe3f90a7589e4ff9e5e8bdf353cdcdfe88764)

osi/src/alarm.cc

diff --git a/osi/src/alarm.cc b/osi/src/alarm.cc
index e1c87fc..9bf9cc5 100644
--- a/osi/src/alarm.cc
+++ b/osi/src/alarm.cc
@@ -71,7 +71,6 @@
   size_t rescheduled_count;
   size_t total_updates;
   period_ms_t last_update_ms;
-  stat_t callback_execution;
   stat_t overdue_scheduling;
   stat_t premature_scheduling;
 } alarm_stats_t;
@@ -155,8 +154,7 @@
 static void callback_dispatch(void* context);
 static bool timer_create_internal(const clockid_t clock_id, timer_t* timer);
 static void update_scheduling_stats(alarm_stats_t* stats, period_ms_t now_ms,
-                                    period_ms_t deadline_ms,
-                                    period_ms_t execution_delta_ms);
+                                    period_ms_t deadline_ms);
 // Registers |queue| for processing alarm callbacks on |thread|.
 // |queue| may not be NULL. |thread| may not be NULL.
 static void alarm_register_processing_queue(fixed_queue_t* queue,
@@ -584,14 +582,12 @@
   std::lock_guard<std::recursive_mutex> cb_lock(*alarm->callback_mutex);
   lock.unlock();
 
-  period_ms_t t0 = now();
-  callback(data);
-  period_ms_t t1 = now();
-
   // Update the statistics
-  CHECK(t1 >= t0);
-  period_ms_t delta = t1 - t0;
-  update_scheduling_stats(&alarm->stats, t0, deadline, delta);
+  update_scheduling_stats(&alarm->stats, now(), deadline);
+
+  // NOTE: Do NOT access "alarm" after the callback, as a safety precaution
+  // in case the callback itself deleted the alarm.
+  callback(data);
 }
 
 static void alarm_ready_mloop(alarm_t* alarm) {
@@ -696,13 +692,10 @@
 }
 
 static void update_scheduling_stats(alarm_stats_t* stats, period_ms_t now_ms,
-                                    period_ms_t deadline_ms,
-                                    period_ms_t execution_delta_ms) {
+                                    period_ms_t deadline_ms) {
   stats->total_updates++;
   stats->last_update_ms = now_ms;
 
-  update_stat(&stats->callback_execution, execution_delta_ms);
-
   if (deadline_ms < now_ms) {
     // Overdue scheduling
     period_ms_t delta_ms = now_ms - deadline_ms;
@@ -749,7 +742,7 @@
     dprintf(fd, "%-51s: %zu / %zu / %zu / %zu\n",
             "    Action counts (sched/resched/exec/cancel)",
             stats->scheduled_count, stats->rescheduled_count,
-            stats->callback_execution.count, stats->canceled_count);
+            stats->total_updates, stats->canceled_count);
 
     dprintf(fd, "%-51s: %zu / %zu\n",
             "    Deviation counts (overdue/premature)",
@@ -761,9 +754,6 @@
             (unsigned long long)alarm->period,
             (long long)(alarm->deadline - just_now));
 
-    dump_stat(fd, &stats->callback_execution,
-              "    Callback execution time in ms (total/max/avg)");
-
     dump_stat(fd, &stats->overdue_scheduling,
               "    Overdue scheduling time in ms (total/max/avg)");