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

[Gerd Hoffmann <kraxel@redhat.com>]

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

  Hi,

> > 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.
> 
> We need a possibility to shrink the buffer after it has been used.
> Especially the queue->buffer.

That works fine.  Test patch attached.

I'm not sure this is the way to go though.  I see alot of growing and
shrinking.  We also do alot of coping (each realloc, but also from
buffer to buffer).

We might be better off redoing the whole buffer management, at least
once we are done with encoding one frame.  Passing on a *pointer* to the
buffer, once sent to the wire just free the buffer.  Allocate a new one
for the next frame.  That way we copy around less data and also don't
have to worry about big unused buffers in the first place ...

cheers,
  Gerd


[-- Attachment #2: 0001-test-buffer-shrink-in-buffer_reserve.patch --]
[-- Type: text/x-patch, Size: 1443 bytes --]

From caa6c3b78595343c651bf4db96dc25db0b163486 Mon Sep 17 00:00:00 2001
From: Gerd Hoffmann <kraxel@redhat.com>
Date: Thu, 3 Sep 2015 13:41:39 +0200
Subject: [PATCH] [test] buffer shrink in buffer_reserve

---
 ui/vnc.c | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/ui/vnc.c b/ui/vnc.c
index aa6b0a5..728cbbe 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -667,6 +667,7 @@ void buffer_init(Buffer *buffer, int fd, const char *name)
 void buffer_reserve(Buffer *buffer, size_t len)
 {
     bool inc = false;
+    bool dec = false;
 
     assert(buffer->name);
     if ((buffer->capacity - buffer->offset) < len) {
@@ -675,9 +676,17 @@ void buffer_reserve(Buffer *buffer, size_t len)
         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);
+    if (((buffer->offset + len) < buffer->capacity >> 4) &&
+        (buffer->capacity > 64 * 1024)) {
+        buffer->capacity >>= 1;
+        buffer->capacity = MAX(buffer->capacity, 64 * 1024);
+        buffer->buffer = g_realloc(buffer->buffer, buffer->capacity);
+        dec = true;
+    }
+    if (inc || dec) {
+        fprintf(stderr, "%s: %-10s: %4zd kB %s\n", __func__,
+                buffer->name, buffer->capacity / 1024,
+                inc ? "+" : "-");
     }
 }
 
-- 
1.8.3.1