Re: [Qemu-devel] [PATCH v3] Optimize record/replay checkpointing for all clocks it applies to

[Paolo Bonzini]

On 18/10/2018 13:16, Artem Pisarenko wrote:
> Removes redundant checkpoints in replay log when there are no expired timers 
> in timers list, associated with corresponding clock (i.e. no rr events 
> associated with current clock value).
> This also improves performance in rr mode.
> 
> Signed-off-by: Artem Pisarenko <address@hidden>
> ---
> 
> Oops, forgot to commit this fix
> 
>     v3:
>     - fixed compiler warning caused non-debug build to fail

We can also move the switch statement to a separate function, it
simplifies the code:

diff --git a/util/qemu-timer.c b/util/qemu-timer.c
index 8a2ad3bce2..3a64ce33d3 100644
--- a/util/qemu-timer.c
+++ b/util/qemu-timer.c
@@ -482,6 +482,26 @@ bool timer_expired(QEMUTimer *timer_head, int64_t
current_time)
     return timer_expired_ns(timer_head, current_time * timer_head->scale);
 }

+static bool timer_checkpoint(QEMUClockType clock)
+{
+    if (replay_mode != REPLAY_MODE_NONE) {
+        switch (clock) {
+        case QEMU_CLOCK_VIRTUAL:
+            return replay_checkpoint(CHECKPOINT_CLOCK_VIRTUAL);
+        case QEMU_CLOCK_HOST:
+            return replay_checkpoint(CHECKPOINT_CLOCK_HOST);
+        case QEMU_CLOCK_VIRTUAL_RT:
+            return replay_checkpoint(CHECKPOINT_CLOCK_VIRTUAL_RT);
+        default:
+            /* QEMU_CLOCK_REALTIME is external to the emulation and does
+             * not need checkpointing.
+             */
+            break;
+        }
+    }
+    return true;
+}
+
 bool timerlist_run_timers(QEMUTimerList *timer_list)
 {
     QEMUTimer *ts;
@@ -489,8 +509,7 @@ bool timerlist_run_timers(QEMUTimerList *timer_list)
     bool progress = false;
     QEMUTimerCB *cb;
     void *opaque;
-    bool need_replay_checkpoint = false;
-    ReplayCheckpoint replay_checkpoint_id;
+    bool need_replay_checkpoint = true;

     if (!atomic_read(&timer_list->active_timers)) {
         return false;
@@ -501,28 +520,6 @@ bool timerlist_run_timers(QEMUTimerList *timer_list)
         goto out;
     }

-    if (replay_mode != REPLAY_MODE_NONE) {
-        /* Postpone actual checkpointing to timer list processing
-         * to properly check if we actually need it.
-         */
-        switch (timer_list->clock->type) {
-        case QEMU_CLOCK_VIRTUAL:
-            need_replay_checkpoint = true;
-            replay_checkpoint_id = CHECKPOINT_CLOCK_VIRTUAL;
-            break;
-        case QEMU_CLOCK_HOST:
-            need_replay_checkpoint = true;
-            replay_checkpoint_id = CHECKPOINT_CLOCK_HOST;
-            break;
-        case QEMU_CLOCK_VIRTUAL_RT:
-            need_replay_checkpoint = true;
-            replay_checkpoint_id = CHECKPOINT_CLOCK_VIRTUAL_RT;
-            break;
-        default:
-            break;
-        }
-    }
-
     /*
      * Extract expired timers from active timers list and and process them,
      * taking into account checkpointing required in rr mode.
@@ -545,11 +542,11 @@ bool timerlist_run_timers(QEMUTimerList *timer_list)
             break;
         }
         if (need_replay_checkpoint
                 && !(ts->attributes & QEMU_TIMER_ATTR_EXTERNAL)) {
             /* once we got here, checkpoint clock only once */
             need_replay_checkpoint = false;
             qemu_mutex_unlock(&timer_list->active_timers_lock);
-            if (!replay_checkpoint(replay_checkpoint_id)) {
+            if (!timer_checkpoint(timer_list->clock->type)) {
                 goto out;
             }
             qemu_mutex_lock(&timer_list->active_timers_lock);


No need to do anything on your part.

Paolo