diff mbox

[2/4] vnc: allow the Buffer to shrink again

Message ID 1441281131.557.56.camel@redhat.com
State New
Headers show

Commit Message

Gerd Hoffmann Sept. 3, 2015, 11:52 a.m. UTC
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

Comments

Peter Lieven Sept. 3, 2015, 2:52 p.m. UTC | #1
Am 03.09.2015 um 13:52 schrieb Gerd Hoffmann:
>    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.

Not completely. queue->buffer may remain big, if you disconnect in the
wrong moment. Maybe there should be a buffer_free if the last client
disconnects?

In general I like your modified patch because the shrinking is slower than
in my version. for 64*1024 you should introduce a macro.

>
> 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 ...

Maybe this should be the permanent solution.

Peter
diff mbox

Patch

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