[Spice-devel] [PATCH spice-gtk 3/6] spice-channel: replace the xmit buf by a queue of SpiceOutMsg-s

[Hans de Goede]

This has a number of advantages:
-It significantly simplifies the code
-It avoids memcpy-ing all the write data one more time
-It avoids malloc / realloc / free of the xmit buffer
 (this gets replaced by gslice alloc / free for the queue elements)

Signed-off-by: Hans de Goede <hdegoede at redhat.com>
---
 gtk/spice-channel-priv.h |    4 +-
 gtk/spice-channel.c      |   99 +++++++++++++++------------------------------
 2 files changed, 34 insertions(+), 69 deletions(-)

diff --git a/gtk/spice-channel-priv.h b/gtk/spice-channel-priv.h
index 8aa0e7c..ed5f3a7 100644
--- a/gtk/spice-channel-priv.h
+++ b/gtk/spice-channel-priv.h
@@ -94,9 +94,7 @@ struct _SpiceChannelPrivate {
     guint                       connect_delayed_id;
 
     struct wait_queue           wait;
-    guint8                      *xmit_buffer;
-    int                         xmit_buffer_capacity;
-    int                         xmit_buffer_size;
+    GQueue                      xmit_queue;
 
     char                        name[16];
     enum spice_channel_state    state;
diff --git a/gtk/spice-channel.c b/gtk/spice-channel.c
index 9e2aee0..5e5f351 100644
--- a/gtk/spice-channel.c
+++ b/gtk/spice-channel.c
@@ -104,6 +104,7 @@ static void spice_channel_init(SpiceChannel *channel)
     c->remote_caps = g_array_new(FALSE, TRUE, sizeof(guint32));
     c->remote_common_caps = g_array_new(FALSE, TRUE, sizeof(guint32));
     spice_channel_set_common_capability(channel, SPICE_COMMON_CAP_PROTOCOL_AUTH_SELECTION);
+    g_queue_init(&c->xmit_queue);
 }
 
 static void spice_channel_constructed(GObject *gobject)
@@ -682,6 +683,25 @@ static void spice_channel_write(SpiceChannel *channel, const void *data, size_t
         spice_channel_flush_wire(channel, data, len);
 }
 
+/* coroutine context */
+static void spice_channel_write_msg(SpiceChannel *channel, SpiceMsgOut *out)
+{
+    uint8_t *data;
+    int free_data;
+    size_t len;
+
+    g_return_if_fail(channel != NULL);
+    g_return_if_fail(out != NULL);
+    g_return_if_fail(channel == out->channel);
+
+    data = spice_marshaller_linearize(out->marshaller, 0, &len, &free_data);
+    /* spice_msg_out_hexdump(out, data, len); */
+    spice_channel_write(channel, data, len);
+
+    if (free_data)
+        free(data);
+}
+
 /*
  * Read at least 1 more byte of data straight off the wire
  * into the requested buffer.
@@ -1526,25 +1546,6 @@ error:
 }
 
 /* system context */
-static void spice_channel_buffered_write(SpiceChannel *channel, const void *data, size_t size)
-{
-    SpiceChannelPrivate *c = channel->priv;
-    size_t left;
-
-    left = c->xmit_buffer_capacity - c->xmit_buffer_size;
-    if (left < size) {
-        c->xmit_buffer_capacity += size + 4095;
-        c->xmit_buffer_capacity &= ~4095;
-
-        c->xmit_buffer = g_realloc(c->xmit_buffer, c->xmit_buffer_capacity);
-    }
-
-    memcpy(&c->xmit_buffer[c->xmit_buffer_size], data, size);
-
-    c->xmit_buffer_size += size;
-}
-
-/* system context */
 /* TODO: we currently flush/wakeup immediately all buffered messages */
 G_GNUC_INTERNAL
 void spice_channel_wakeup(SpiceChannel *channel)
@@ -1564,9 +1565,7 @@ gboolean spice_channel_get_read_only(SpiceChannel *channel)
    system context if @buffered is TRUE */
 static void spice_channel_send_msg(SpiceChannel *channel, SpiceMsgOut *out, gboolean buffered)
 {
-    uint8_t *data;
-    int free_data;
-    size_t len;
+    SpiceChannelPrivate *c = channel->priv;
 
     g_return_if_fail(channel != NULL);
     g_return_if_fail(out != NULL);
@@ -1577,16 +1576,12 @@ static void spice_channel_send_msg(SpiceChannel *channel, SpiceMsgOut *out, gboo
         return;
     }
 
-    data = spice_marshaller_linearize(out->marshaller, 0,
-                                      &len, &free_data);
-    /* spice_msg_out_hexdump(out, data, len); */
-    if (buffered)
-        spice_channel_buffered_write(channel, data, len);
-    else
-        spice_channel_write(channel, data, len);
-
-    if (free_data)
-        free(data);
+    if (buffered) {
+        spice_msg_out_ref(out);
+        g_queue_push_tail(&c->xmit_queue, out);
+    } else {
+        spice_channel_write_msg(channel, out);
+    }
 }
 
 /* coroutine context */
@@ -1804,35 +1799,11 @@ void spice_channel_destroy(SpiceChannel *channel)
 static void spice_channel_iterate_write(SpiceChannel *channel)
 {
     SpiceChannelPrivate *c = channel->priv;
+    SpiceMsgOut *out;
 
-    while (c->xmit_buffer_size) {
-        /*
-         * Take ownership of the buffer, so that if spice_channel_write calls
-         * g_io_wait and thus yields to the main context, and that then calls
-         * spice_channel_buffered_write it does not mess with the buffer
-         * being written out.
-         */
-        guint8 *buffer = c->xmit_buffer;
-        int size = c->xmit_buffer_size;
-        int capacity = c->xmit_buffer_capacity;
-
-        c->xmit_buffer = NULL;
-        c->xmit_buffer_size = 0;
-        c->xmit_buffer_capacity = 0;
-
-        spice_channel_write(channel, buffer, size);
-
-        /*
-         * If no write has been done in the mean time, we can return the buffer
-         * so that it can be re-used.
-         */
-        if (c->xmit_buffer == NULL) {
-            c->xmit_buffer = buffer;
-            c->xmit_buffer_capacity = capacity;
-            c->xmit_buffer_size = 0;
-        } else {
-            g_free(buffer);
-        }
+    while ((out = g_queue_pop_head(&c->xmit_queue))) {
+        spice_channel_write_msg(channel, out);
+        spice_msg_out_unref(out);
     }
 }
 
@@ -2213,12 +2184,8 @@ static void channel_disconnect(SpiceChannel *channel)
     c->peer_msg = NULL;
     c->peer_pos = 0;
 
-    if (c->xmit_buffer) {
-        g_free(c->xmit_buffer);
-        c->xmit_buffer = NULL;
-        c->xmit_buffer_size = 0;
-        c->xmit_buffer_capacity = 0;
-    }
+    g_queue_foreach(&c->xmit_queue, (GFunc)spice_msg_out_unref, NULL);
+    g_queue_clear(&c->xmit_queue);
 
     g_array_set_size(c->remote_common_caps, 0);
     g_array_set_size(c->remote_caps, 0);
-- 
1.7.7.4