[Spice-devel] [PATCH spice-gtk 09/14] channel: do not reenter the mainloop at every iteration

[Marc-André Lureau]

From: Marc-André Lureau <marcandre.lureau at gmail.com>

The current coroutine channel_iterate() creates a GSource for the socket
reenters the mainloop waiting for IO condition. This is really heavy
usage of mainloop showing up in system time and user space
profiling (10% of CPU time spent in mainloop overhead). Instead flush
all pending input messages before switching context and reentering main
loop.

This is the kind of results I get with a replay:

before:

       2179,440455 task-clock                #    0,629 CPUs utilized
             3 580 context-switches          #    0,002 M/sec
               207 cpu-migrations            #    0,095 K/sec
            48 909 page-faults               #    0,022 M/sec
     7 362 442 301 cycles                    #    3,378 GHz
     5 256 957 520 stalled-cycles-frontend   #   71,40% frontend cycles
             idle
   <not supported> stalled-cycles-backend
     5 460 266 981 instructions              #    0,74  insns per cycle
                                             #    0,96  stalled cycles
             per insn
       982 749 523 branches                  #  450,918 M/sec
        20 745 453 branch-misses             #    2,11% of all branches

       3,463422087 seconds time elapsed

after:

       1741,651383 task-clock                #    0,522 CPUs utilized
             5 093 context-switches          #    0,003 M/sec
               137 cpu-migrations            #    0,079 K/sec
            49 033 page-faults               #    0,028 M/sec
     5 874 567 974 cycles                    #    3,373 GHz
     4 247 059 288 stalled-cycles-frontend   #   72,30% frontend cycles
             idle
   <not supported> stalled-cycles-backend
     4 419 666 346 instructions              #    0,75  insns per cycle
                                             #    0,96  stalled cycles
             per insn
       776 972 366 branches                  #  446,112 M/sec
        17 862 170 branch-misses             #    2,30% of all branches

       3,337472053 seconds time elapsed
---
 gtk/spice-channel.c | 59 +++++++++++++++++++++++++----------------------------
 1 file changed, 28 insertions(+), 31 deletions(-)

diff --git a/gtk/spice-channel.c b/gtk/spice-channel.c
index 55c8325..f6a691e 100644
--- a/gtk/spice-channel.c
+++ b/gtk/spice-channel.c
@@ -2080,10 +2080,24 @@ static void spice_channel_iterate_read(SpiceChannel *channel)
 {
     SpiceChannelPrivate *c = channel->priv;
 
-    g_return_if_fail(c->state != SPICE_CHANNEL_STATE_MIGRATING);
+    g_coroutine_socket_wait(&c->coroutine, c->sock, G_IO_IN);
+
+    /* treat all incoming data (block on message completion) */
+    while (!c->has_error &&
+           c->state != SPICE_CHANNEL_STATE_MIGRATING &&
+           g_socket_condition_check(c->sock, G_IO_IN) & G_IO_IN) {
+
+        do
+            spice_channel_recv_msg(channel,
+                                   (handler_msg_in)SPICE_CHANNEL_GET_CLASS(channel)->handle_msg, NULL);
+#if HAVE_SASL
+            /* flush the sasl buffer too */
+        while (c->sasl_decoded != NULL);
+#else
+        while (FALSE);
+#endif
+    }
 
-    spice_channel_recv_msg(channel,
-        (handler_msg_in)SPICE_CHANNEL_GET_CLASS(channel)->handle_msg, NULL);
 }
 
 static gboolean wait_migration(gpointer data)
@@ -2105,37 +2119,20 @@ static gboolean spice_channel_iterate(SpiceChannel *channel)
     SpiceChannelPrivate *c = channel->priv;
     GIOCondition ret;
 
-    do {
-        if (c->state == SPICE_CHANNEL_STATE_MIGRATING &&
-            !g_coroutine_condition_wait(&c->coroutine, wait_migration, channel))
-                CHANNEL_DEBUG(channel, "migration wait cancelled");
-
-        if (c->has_error) {
-            CHANNEL_DEBUG(channel, "channel has error, breaking loop");
-            return FALSE;
-        }
-
-        SPICE_CHANNEL_GET_CLASS(channel)->iterate_write(channel);
-
-        ret = g_coroutine_socket_wait(&c->coroutine, c->sock,
-            c->state != SPICE_CHANNEL_STATE_MIGRATING ? G_IO_IN : 0);
-
-#ifdef WIN32
-        /* FIXME: windows gsocket is buggy, it doesn't return correct condition... */
-        ret = g_socket_condition_check(c->sock, G_IO_IN);
-#endif
-    } while (ret == 0); /* ret == 0 means no IO condition, but woken up */
+    if (c->state == SPICE_CHANNEL_STATE_MIGRATING &&
+        !g_coroutine_condition_wait(&c->coroutine, wait_migration, channel))
+        CHANNEL_DEBUG(channel, "migration wait cancelled");
 
-    if (ret & G_IO_IN) {
-        do
-            SPICE_CHANNEL_GET_CLASS(channel)->iterate_read(channel);
-#if HAVE_SASL
-        while (c->sasl_decoded != NULL);
-#else
-        while (FALSE);
-#endif
+    if (c->has_error) {
+        CHANNEL_DEBUG(channel, "channel has error, breaking loop");
+        return FALSE;
     }
 
+    /* flush any pending write and read */
+    SPICE_CHANNEL_GET_CLASS(channel)->iterate_write(channel);
+    SPICE_CHANNEL_GET_CLASS(channel)->iterate_read(channel);
+
+    ret = g_socket_condition_check(c->sock, G_IO_IN | G_IO_ERR | G_IO_HUP);
     if (c->state > SPICE_CHANNEL_STATE_CONNECTING &&
         ret & (G_IO_ERR|G_IO_HUP)) {
         SPICE_DEBUG("got socket error: %d", ret);
-- 
1.8.3.1