Re: [Qemu-devel] [PATCH 2/4] vnc: allow the Buffer to shrink again

[Gerd Hoffmann <kraxel@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 1342 bytes --]

On Do, 2015-08-27 at 11:39 +0100, Daniel P. Berrange wrote:
> On Thu, Aug 27, 2015 at 12:18:52PM +0200, Peter Lieven wrote:
> > currently the Buffer can only grow. This increases Qemu memory footprint
> > dramatically since normally the biggest VNC updates are at connection time.
> > But also after a VNC session has terminated there is one persistent buffer
> > in queue->buffer which I have seen to increase to over 100MB and it is never
> > getting smaller again.
> 
> Do you have any idea what caused the buffer to increase to 100MB in size
> in the first place ? I would expect a full screen update would cause the
> biggest buffer usage, and even for a 1920x1140 screen that should not
> be anywhere near 100MB in size. IOW, i'm wondering if the 100MB usage
> is symptomatic of a more serious bug somewhere else in the VNC code
> that you're just masking you reducing buffer size afterwards.

Cooked up a buffer stats patch (attached).
Buffer sizes are nowhere near 100MB for me (as expected by Daniel).
Individual buffers are in the 1 -> 4 MB range (1920x1080), even summed
up this is more like 10 not 100 MB.

So, big question remains why they are that big for you?

Beside that I think it makes sense to have the shrinking logic in
buffer_reserve too so we don't have to add buffer_shrink calls all over
the place.

cheers,
  Gerd


[-- Attachment #2: 0001-debug-vnc-buffer-stats.patch --]
[-- Type: text/x-patch, Size: 3442 bytes --]

From e5d9353d02983db0081e44026767adf9f5a72832 Mon Sep 17 00:00:00 2001
From: Gerd Hoffmann <kraxel@redhat.com>
Date: Thu, 3 Sep 2015 11:44:18 +0200
Subject: [PATCH] [debug] vnc buffer stats

---
 ui/vnc-jobs.c |  1 +
 ui/vnc.c      | 38 ++++++++++++++++++++++++++++++++++++++
 ui/vnc.h      |  2 ++
 3 files changed, 41 insertions(+)

diff --git a/ui/vnc-jobs.c b/ui/vnc-jobs.c
index 22c9abc..1338771 100644
--- a/ui/vnc-jobs.c
+++ b/ui/vnc-jobs.c
@@ -302,6 +302,7 @@ static VncJobQueue *vnc_queue_init(void)
 
     qemu_cond_init(&queue->cond);
     qemu_mutex_init(&queue->mutex);
+    buffer_init(&queue->buffer, 0, "queue");
     QTAILQ_INIT(&queue->jobs);
     return queue;
 }
diff --git a/ui/vnc.c b/ui/vnc.c
index 7a17954..59be765 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -655,12 +655,29 @@ void vnc_framebuffer_update(VncState *vs, int x, int y, int w, int h,
 
 #define BUFFER_MIN_INIT_SIZE 4096
 
+void buffer_init(Buffer *buffer, int fd, const char *name)
+{
+    if (fd) {
+        buffer->name = g_strdup_printf("%d/%s", fd, name);
+    } else {
+        buffer->name = g_strdup(name);
+    }
+}
+
 void buffer_reserve(Buffer *buffer, size_t len)
 {
+    bool inc = false;
+
+    assert(buffer->name);
     if ((buffer->capacity - buffer->offset) < len) {
         buffer->capacity = pow2ceil(buffer->capacity + len);
         buffer->capacity = MAX(buffer->capacity, BUFFER_MIN_INIT_SIZE);
         buffer->buffer = g_realloc(buffer->buffer, buffer->capacity);
+        inc = true;
+    }
+    if (inc) {
+        fprintf(stderr, "%s: %-10s: %4zd kB\n", __func__,
+                buffer->name, buffer->capacity / 1024);
     }
 }
 
@@ -681,6 +698,7 @@ void buffer_reset(Buffer *buffer)
 
 void buffer_free(Buffer *buffer)
 {
+    fprintf(stderr, "%s: %s\n", __func__, buffer->name);
     g_free(buffer->buffer);
     buffer->offset = 0;
     buffer->capacity = 0;
@@ -3027,6 +3045,26 @@ static void vnc_connect(VncDisplay *vd, int csock,
     vs->csock = csock;
     vs->vd = vd;
 
+    buffer_init(&vs->input, vs->csock, "in");
+    buffer_init(&vs->output, vs->csock, "out");
+    buffer_init(&vs->ws_input, vs->csock, "w-in");
+    buffer_init(&vs->ws_output, vs->csock, "w-out");
+    buffer_init(&vs->jobs_buffer, vs->csock, "jobs");
+
+    buffer_init(&vs->tight.tight, vs->csock, "t-tight");
+    buffer_init(&vs->tight.zlib, vs->csock, "t-zlib");
+    buffer_init(&vs->tight.gradient, vs->csock, "t-gradient");
+#ifdef CONFIG_VNC_JPEG
+    buffer_init(&vs->tight.jpeg, vs->csock, "t-jpeg");
+#endif
+#ifdef CONFIG_VNC_PNG
+    buffer_init(&vs->tight.png, vs->csock, "t-png");
+#endif
+    buffer_init(&vs->zlib.zlib, vs->csock, "zlib");
+    buffer_init(&vs->zrle.zrle, vs->csock, "z-zrle");
+    buffer_init(&vs->zrle.fb, vs->csock, "z-fb");
+    buffer_init(&vs->zrle.zlib, vs->csock, "z-zlib");
+
     if (skipauth) {
 	vs->auth = VNC_AUTH_NONE;
 	vs->subauth = VNC_AUTH_INVALID;
diff --git a/ui/vnc.h b/ui/vnc.h
index 814d720..95878c3 100644
--- a/ui/vnc.h
+++ b/ui/vnc.h
@@ -57,6 +57,7 @@
 
 typedef struct Buffer
 {
+    char *name;
     size_t capacity;
     size_t offset;
     uint8_t *buffer;
@@ -539,6 +540,7 @@ void start_client_init(VncState *vs);
 void start_auth_vnc(VncState *vs);
 
 /* Buffer management */
+void buffer_init(Buffer *buffer, int fd, const char *name);
 void buffer_reserve(Buffer *buffer, size_t len);
 void buffer_reset(Buffer *buffer);
 void buffer_free(Buffer *buffer);
-- 
1.8.3.1