[Qemu-devel] [PATCH 0/4] spice: improve vga mode performance

[Qemu-devel] [PATCH 0/4] spice: improve vga mode performance

[Gerd Hoffmann]

  Hi,

This patch series makes spice be more clever on screen updates when in
vga mode (i.e. without qxl guest driver loaded or when using a non-qxl
vga card).  qemu keeps a copy of the screen with the content forwarded
to spice-server, then on updates it goes compare the guest screen with
the copy to figure what has really changed.

cheers,
  Gerd

Gerd Hoffmann (4):
  spice: switch to queue for vga mode updates
  spice: split qemu_spice_create_update
  spice: add screen mirror
  spice: send updates only for changed screen content

 hw/qxl.c           |    6 +-
 ui/spice-display.c |  135 ++++++++++++++++++++++++++++++++++++++++------------
 ui/spice-display.h |    4 +-
 3 files changed, 111 insertions(+), 34 deletions(-)

[Qemu-devel] [PATCH 1/4] spice: switch to queue for vga mode updates

[Gerd Hoffmann]

---
 hw/qxl.c           |    6 +++---
 ui/spice-display.c |   25 ++++++++++++++-----------
 ui/spice-display.h |    3 ++-
 3 files changed, 19 insertions(+), 15 deletions(-)

diff --git a/hw/qxl.c b/hw/qxl.c
index b726c19..833cd77 100644
--- a/hw/qxl.c
+++ b/hw/qxl.c
@@ -597,9 +597,9 @@ static int interface_get_command(QXLInstance *sin, struct QXLCommandExt *ext)
     case QXL_MODE_VGA:
         ret = false;
         qemu_mutex_lock(&qxl->ssd.lock);
-        if (qxl->ssd.update != NULL) {
-            update = qxl->ssd.update;
-            qxl->ssd.update = NULL;
+        update = QTAILQ_FIRST(&qxl->ssd.updates);
+        if (update != NULL) {
+            QTAILQ_REMOVE(&qxl->ssd.updates, update, next);
             *ext = update->ext;
             ret = true;
         }
diff --git a/ui/spice-display.c b/ui/spice-display.c
index 99bc665..59c5fd7 100644
--- a/ui/spice-display.c
+++ b/ui/spice-display.c
@@ -164,7 +164,7 @@ int qemu_spice_display_is_running(SimpleSpiceDisplay *ssd)
 #endif
 }
 
-static SimpleSpiceUpdate *qemu_spice_create_update(SimpleSpiceDisplay *ssd)
+static void qemu_spice_create_update(SimpleSpiceDisplay *ssd)
 {
     SimpleSpiceUpdate *update;
     QXLDrawable *drawable;
@@ -175,7 +175,7 @@ static SimpleSpiceUpdate *qemu_spice_create_update(SimpleSpiceDisplay *ssd)
     struct timespec time_space;
 
     if (qemu_spice_rect_is_empty(&ssd->dirty)) {
-        return NULL;
+        return;
     };
 
     trace_qemu_spice_create_update(
@@ -239,7 +239,7 @@ static SimpleSpiceUpdate *qemu_spice_create_update(SimpleSpiceDisplay *ssd)
     cmd->data = (uintptr_t)drawable;
 
     memset(&ssd->dirty, 0, sizeof(ssd->dirty));
-    return update;
+    QTAILQ_INSERT_TAIL(&ssd->updates, update, next);
 }
 
 /*
@@ -315,6 +315,7 @@ void qemu_spice_display_init_common(SimpleSpiceDisplay *ssd, DisplayState *ds)
 {
     ssd->ds = ds;
     qemu_mutex_init(&ssd->lock);
+    QTAILQ_INIT(&ssd->updates);
     ssd->mouse_x = -1;
     ssd->mouse_y = -1;
     if (ssd->num_surfaces == 0) {
@@ -345,6 +346,8 @@ void qemu_spice_display_update(SimpleSpiceDisplay *ssd,
 
 void qemu_spice_display_resize(SimpleSpiceDisplay *ssd)
 {
+    SimpleSpiceUpdate *update;
+
     dprint(1, "%s:\n", __FUNCTION__);
 
     memset(&ssd->dirty, 0, sizeof(ssd->dirty));
@@ -352,9 +355,9 @@ void qemu_spice_display_resize(SimpleSpiceDisplay *ssd)
     ssd->conv = NULL;
 
     qemu_mutex_lock(&ssd->lock);
-    if (ssd->update != NULL) {
-        qemu_spice_destroy_update(ssd, ssd->update);
-        ssd->update = NULL;
+    while ((update = QTAILQ_FIRST(&ssd->updates)) != NULL) {
+        QTAILQ_REMOVE(&ssd->updates, update, next);
+        qemu_spice_destroy_update(ssd, update);
     }
     qemu_mutex_unlock(&ssd->lock);
     qemu_spice_destroy_host_primary(ssd);
@@ -384,8 +387,8 @@ void qemu_spice_display_refresh(SimpleSpiceDisplay *ssd)
     vga_hw_update();
 
     qemu_mutex_lock(&ssd->lock);
-    if (ssd->update == NULL) {
-        ssd->update = qemu_spice_create_update(ssd);
+    if (QTAILQ_EMPTY(&ssd->updates)) {
+        qemu_spice_create_update(ssd);
         ssd->notify++;
     }
     qemu_spice_cursor_refresh_unlocked(ssd);
@@ -442,9 +445,9 @@ static int interface_get_command(QXLInstance *sin, struct QXLCommandExt *ext)
     dprint(3, "%s:\n", __FUNCTION__);
 
     qemu_mutex_lock(&ssd->lock);
-    if (ssd->update != NULL) {
-        update = ssd->update;
-        ssd->update = NULL;
+    update = QTAILQ_FIRST(&ssd->updates);
+    if (update != NULL) {
+        QTAILQ_REMOVE(&ssd->updates, update, next);
         *ext = update->ext;
         ret = true;
     }
diff --git a/ui/spice-display.h b/ui/spice-display.h
index 512ab78..3fcb6fe 100644
--- a/ui/spice-display.h
+++ b/ui/spice-display.h
@@ -92,7 +92,7 @@ struct SimpleSpiceDisplay {
      * to them must be protected by the lock.
      */
     QemuMutex lock;
-    SimpleSpiceUpdate *update;
+    QTAILQ_HEAD(, SimpleSpiceUpdate) updates;
     QEMUCursor *cursor;
     int mouse_x, mouse_y;
 };
@@ -102,6 +102,7 @@ struct SimpleSpiceUpdate {
     QXLImage image;
     QXLCommandExt ext;
     uint8_t *bitmap;
+    QTAILQ_ENTRY(SimpleSpiceUpdate) next;
 };
 
 int qemu_spice_rect_is_empty(const QXLRect* r);
-- 
1.7.1

[Qemu-devel] [PATCH 2/4] spice: split qemu_spice_create_update

[Gerd Hoffmann]

Creating one function which creates a single update for a given
rectangle.  And one (for now) pretty simple wrapper around it to
queue up screen updates for the dirty region.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 ui/spice-display.c |   29 +++++++++++++++++------------
 1 files changed, 17 insertions(+), 12 deletions(-)

diff --git a/ui/spice-display.c b/ui/spice-display.c
index 59c5fd7..bc2f207 100644
--- a/ui/spice-display.c
+++ b/ui/spice-display.c
@@ -164,7 +164,8 @@ int qemu_spice_display_is_running(SimpleSpiceDisplay *ssd)
 #endif
 }
 
-static void qemu_spice_create_update(SimpleSpiceDisplay *ssd)
+static void qemu_spice_create_one_update(SimpleSpiceDisplay *ssd,
+                                         QXLRect *rect)
 {
     SimpleSpiceUpdate *update;
     QXLDrawable *drawable;
@@ -174,21 +175,17 @@ static void qemu_spice_create_update(SimpleSpiceDisplay *ssd)
     int by, bw, bh;
     struct timespec time_space;
 
-    if (qemu_spice_rect_is_empty(&ssd->dirty)) {
-        return;
-    };
-
     trace_qemu_spice_create_update(
-           ssd->dirty.left, ssd->dirty.right,
-           ssd->dirty.top, ssd->dirty.bottom);
+           rect->left, rect->right,
+           rect->top, rect->bottom);
 
     update   = g_malloc0(sizeof(*update));
     drawable = &update->drawable;
     image    = &update->image;
     cmd      = &update->ext.cmd;
 
-    bw       = ssd->dirty.right - ssd->dirty.left;
-    bh       = ssd->dirty.bottom - ssd->dirty.top;
+    bw       = rect->right - rect->left;
+    bh       = rect->bottom - rect->top;
     update->bitmap = g_malloc(bw * bh * 4);
 
     drawable->bbox            = ssd->dirty;
@@ -226,8 +223,8 @@ static void qemu_spice_create_update(SimpleSpiceDisplay *ssd)
     }
 
     src = ds_get_data(ssd->ds) +
-        ssd->dirty.top * ds_get_linesize(ssd->ds) +
-        ssd->dirty.left * ds_get_bytes_per_pixel(ssd->ds);
+        rect->top * ds_get_linesize(ssd->ds) +
+        rect->left * ds_get_bytes_per_pixel(ssd->ds);
     dst = update->bitmap;
     for (by = 0; by < bh; by++) {
         qemu_pf_conv_run(ssd->conv, dst, src, bw);
@@ -238,10 +235,18 @@ static void qemu_spice_create_update(SimpleSpiceDisplay *ssd)
     cmd->type = QXL_CMD_DRAW;
     cmd->data = (uintptr_t)drawable;
 
-    memset(&ssd->dirty, 0, sizeof(ssd->dirty));
     QTAILQ_INSERT_TAIL(&ssd->updates, update, next);
 }
 
+static void qemu_spice_create_update(SimpleSpiceDisplay *ssd)
+{
+    if (qemu_spice_rect_is_empty(&ssd->dirty)) {
+        return;
+    };
+    qemu_spice_create_one_update(ssd, &ssd->dirty);
+    memset(&ssd->dirty, 0, sizeof(ssd->dirty));
+}
+
 /*
  * Called from spice server thread context (via interface_release_ressource)
  * We do *not* hold the global qemu mutex here, so extra care is needed
-- 
1.7.1

[Qemu-devel] [PATCH 3/4] spice: add screen mirror

[Gerd Hoffmann]

Create a screen mirror, keep there a copy of the most recent update
passed on to spice-server.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 ui/spice-display.c |   32 ++++++++++++++++++++++----------
 ui/spice-display.h |    1 +
 2 files changed, 23 insertions(+), 10 deletions(-)

diff --git a/ui/spice-display.c b/ui/spice-display.c
index bc2f207..6f87099 100644
--- a/ui/spice-display.c
+++ b/ui/spice-display.c
@@ -171,8 +171,8 @@ static void qemu_spice_create_one_update(SimpleSpiceDisplay *ssd,
     QXLDrawable *drawable;
     QXLImage *image;
     QXLCommand *cmd;
-    uint8_t *src, *dst;
-    int by, bw, bh;
+    uint8_t *src, *mirror, *dst;
+    int by, bw, bh, offset, bytes;
     struct timespec time_space;
 
     trace_qemu_spice_create_update(
@@ -216,19 +216,18 @@ static void qemu_spice_create_one_update(SimpleSpiceDisplay *ssd,
     image->bitmap.palette = 0;
     image->bitmap.format = SPICE_BITMAP_FMT_32BIT;
 
-    if (ssd->conv == NULL) {
-        PixelFormat dst = qemu_default_pixelformat(32);
-        ssd->conv = qemu_pf_conv_get(&dst, &ssd->ds->surface->pf);
-        assert(ssd->conv);
-    }
-
-    src = ds_get_data(ssd->ds) +
+    offset =
         rect->top * ds_get_linesize(ssd->ds) +
         rect->left * ds_get_bytes_per_pixel(ssd->ds);
+    bytes = ds_get_bytes_per_pixel(ssd->ds) * bw;
+    src = ds_get_data(ssd->ds) + offset;
+    mirror = ssd->ds_mirror + offset;
     dst = update->bitmap;
     for (by = 0; by < bh; by++) {
-        qemu_pf_conv_run(ssd->conv, dst, src, bw);
+        memcpy(mirror, src, bytes);
+        qemu_pf_conv_run(ssd->conv, dst, mirror, bw);
         src += ds_get_linesize(ssd->ds);
+        mirror += ds_get_linesize(ssd->ds);
         dst += image->bitmap.stride;
     }
 
@@ -243,6 +242,17 @@ static void qemu_spice_create_update(SimpleSpiceDisplay *ssd)
     if (qemu_spice_rect_is_empty(&ssd->dirty)) {
         return;
     };
+
+    if (ssd->conv == NULL) {
+        PixelFormat dst = qemu_default_pixelformat(32);
+        ssd->conv = qemu_pf_conv_get(&dst, &ssd->ds->surface->pf);
+        assert(ssd->conv);
+    }
+    if (ssd->ds_mirror == NULL) {
+        int size = ds_get_height(ssd->ds) * ds_get_linesize(ssd->ds);
+        ssd->ds_mirror = g_malloc0(size);
+    }
+
     qemu_spice_create_one_update(ssd, &ssd->dirty);
     memset(&ssd->dirty, 0, sizeof(ssd->dirty));
 }
@@ -358,6 +368,8 @@ void qemu_spice_display_resize(SimpleSpiceDisplay *ssd)
     memset(&ssd->dirty, 0, sizeof(ssd->dirty));
     qemu_pf_conv_put(ssd->conv);
     ssd->conv = NULL;
+    g_free(ssd->ds_mirror);
+    ssd->ds_mirror = NULL;
 
     qemu_mutex_lock(&ssd->lock);
     while ((update = QTAILQ_FIRST(&ssd->updates)) != NULL) {
diff --git a/ui/spice-display.h b/ui/spice-display.h
index 3fcb6fe..dea41c1 100644
--- a/ui/spice-display.h
+++ b/ui/spice-display.h
@@ -72,6 +72,7 @@ typedef struct SimpleSpiceUpdate SimpleSpiceUpdate;
 
 struct SimpleSpiceDisplay {
     DisplayState *ds;
+    uint8_t *ds_mirror;
     void *buf;
     int bufsize;
     QXLWorker *worker;
-- 
1.7.1

[Qemu-devel] [PATCH 4/4] spice: send updates only for changed screen content

[Gerd Hoffmann]

when creating screen updates go compare the current guest screen
against the mirror (which holds the most recent update sent), then
only create updates for the screen areas which did actually change.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 ui/spice-display.c |   55 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 55 insertions(+), 0 deletions(-)

diff --git a/ui/spice-display.c b/ui/spice-display.c
index 6f87099..ea87d82 100644
--- a/ui/spice-display.c
+++ b/ui/spice-display.c
@@ -239,6 +239,13 @@ static void qemu_spice_create_one_update(SimpleSpiceDisplay *ssd,
 
 static void qemu_spice_create_update(SimpleSpiceDisplay *ssd)
 {
+    static const int blksize = 42;
+    int blocks = (ds_get_width(ssd->ds) + blksize - 1) / blksize;
+    int dirty_top[blocks];
+    int y, yoff, x, xoff, blk, bw;
+    int bpp = ds_get_bytes_per_pixel(ssd->ds);
+    uint8_t *guest, *mirror;
+
     if (qemu_spice_rect_is_empty(&ssd->dirty)) {
         return;
     };
@@ -253,6 +260,54 @@ static void qemu_spice_create_update(SimpleSpiceDisplay *ssd)
         ssd->ds_mirror = g_malloc0(size);
     }
 
+    for (blk = 0; blk < blocks; blk++) {
+        dirty_top[blk] = -1;
+    }
+
+    guest = ds_get_data(ssd->ds);
+    mirror = ssd->ds_mirror;
+    for (y = ssd->dirty.top; y < ssd->dirty.bottom; y++) {
+        yoff = y * ds_get_linesize(ssd->ds);
+        for (x = ssd->dirty.left; x < ssd->dirty.right; x += blksize) {
+            xoff = x * bpp;
+            blk = x / blksize;
+            bw = MIN(blksize, ssd->dirty.right - x);
+            if (memcmp(guest + yoff + xoff,
+                       mirror + yoff + xoff,
+                       bw * bpp) == 0) {
+                if (dirty_top[blk] != -1) {
+                    QXLRect update = {
+                        .top    = dirty_top[blk],
+                        .bottom = y,
+                        .left   = x,
+                        .right  = x + bw,
+                    };
+                    qemu_spice_create_one_update(ssd, &update);
+                    dirty_top[blk] = -1;
+                }
+            } else {
+                if (dirty_top[blk] == -1) {
+                    dirty_top[blk] = y;
+                }
+            }
+        }
+    }
+
+    for (x = ssd->dirty.left; x < ssd->dirty.right; x += blksize) {
+        blk = x / blksize;
+        bw = MIN(blksize, ssd->dirty.right - x);
+        if (dirty_top[blk] != -1) {
+            QXLRect update = {
+                .top    = dirty_top[blk],
+                .bottom = ssd->dirty.bottom,
+                .left   = x,
+                .right  = x + bw,
+            };
+            qemu_spice_create_one_update(ssd, &update);
+            dirty_top[blk] = -1;
+        }
+    }
+
     qemu_spice_create_one_update(ssd, &ssd->dirty);
     memset(&ssd->dirty, 0, sizeof(ssd->dirty));
 }
-- 
1.7.1