* [Qemu-devel] [PATCH 0/4] VNC server memory savings
@ 2015-08-27 10:18 Peter Lieven
2015-08-27 10:18 ` [Qemu-devel] [PATCH 1/4] vnc: make the Buffer capacity increase in powers of two Peter Lieven
` (3 more replies)
0 siblings, 4 replies; 20+ messages in thread
From: Peter Lieven @ 2015-08-27 10:18 UTC (permalink / raw)
To: qemu-devel; +Cc: Peter Lieven, kraxel
While debugging increased memory footprint of Qemu (since qemu-kvm-1.2.0)
I found some culprits in the VNC code. This series includes some
first optimizations.
Peter Lieven (4):
vnc: make the Buffer capacity increase in powers of two
vnc: allow the Buffer to shrink again
vnc-jobs: move buffer_reset to vnc_async_encoding_end
vnc: destroy server surface if no client is connected
ui/vnc-jobs.c | 3 +--
ui/vnc.c | 36 +++++++++++++++++++++++++++++++++---
2 files changed, 34 insertions(+), 5 deletions(-)
--
1.9.1
^ permalink raw reply [flat|nested] 20+ messages in thread* [Qemu-devel] [PATCH 1/4] vnc: make the Buffer capacity increase in powers of two 2015-08-27 10:18 [Qemu-devel] [PATCH 0/4] VNC server memory savings Peter Lieven @ 2015-08-27 10:18 ` Peter Lieven 2015-09-03 8:56 ` Gerd Hoffmann 2015-08-27 10:18 ` [Qemu-devel] [PATCH 2/4] vnc: allow the Buffer to shrink again Peter Lieven ` (2 subsequent siblings) 3 siblings, 1 reply; 20+ messages in thread From: Peter Lieven @ 2015-08-27 10:18 UTC (permalink / raw) To: qemu-devel; +Cc: Peter Lieven, kraxel This makes sure the number of reallocs is in O(log N). Signed-off-by: Peter Lieven <pl@kamp.de> --- ui/vnc.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/ui/vnc.c b/ui/vnc.c index caf82f5..8cfd2d8 100644 --- a/ui/vnc.c +++ b/ui/vnc.c @@ -653,10 +653,13 @@ void vnc_framebuffer_update(VncState *vs, int x, int y, int w, int h, vnc_write_s32(vs, encoding); } +#define BUFFER_MIN_INIT_SIZE 4096 + void buffer_reserve(Buffer *buffer, size_t len) { if ((buffer->capacity - buffer->offset) < len) { - buffer->capacity += (len + 1024); + buffer->capacity = pow2ceil(buffer->capacity + len); + buffer->capacity = MAX(buffer->capacity, BUFFER_MIN_INIT_SIZE); buffer->buffer = g_realloc(buffer->buffer, buffer->capacity); } } -- 1.9.1 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [Qemu-devel] [PATCH 1/4] vnc: make the Buffer capacity increase in powers of two 2015-08-27 10:18 ` [Qemu-devel] [PATCH 1/4] vnc: make the Buffer capacity increase in powers of two Peter Lieven @ 2015-09-03 8:56 ` Gerd Hoffmann 2015-09-03 10:29 ` Peter Lieven 0 siblings, 1 reply; 20+ messages in thread From: Gerd Hoffmann @ 2015-09-03 8:56 UTC (permalink / raw) To: Peter Lieven; +Cc: qemu-devel On Do, 2015-08-27 at 12:18 +0200, Peter Lieven wrote: > This makes sure the number of reallocs is in O(log N). Looks good, applied. ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Qemu-devel] [PATCH 1/4] vnc: make the Buffer capacity increase in powers of two 2015-09-03 8:56 ` Gerd Hoffmann @ 2015-09-03 10:29 ` Peter Lieven 2015-09-03 10:47 ` Gerd Hoffmann 0 siblings, 1 reply; 20+ messages in thread From: Peter Lieven @ 2015-09-03 10:29 UTC (permalink / raw) To: Gerd Hoffmann; +Cc: qemu-devel Am 03.09.2015 um 10:56 schrieb Gerd Hoffmann: > On Do, 2015-08-27 at 12:18 +0200, Peter Lieven wrote: >> This makes sure the number of reallocs is in O(log N). > Looks good, applied. > I think there is a small error. The new capacity should be calculated as follows: diff --git a/ui/vnc.c b/ui/vnc.c index 8cfd2d8..79d3ff3 100644 --- a/ui/vnc.c +++ b/ui/vnc.c @@ -658,7 +658,7 @@ void vnc_framebuffer_update(VncState *vs, int x, int y, int w, int h, void buffer_reserve(Buffer *buffer, size_t len) { if ((buffer->capacity - buffer->offset) < len) { - buffer->capacity = pow2ceil(buffer->capacity + len); + buffer->capacity = pow2ceil(buffer->offset + len); buffer->capacity = MAX(buffer->capacity, BUFFER_MIN_INIT_SIZE); buffer->buffer = g_realloc(buffer->buffer, buffer->capacity); } BR, Peter ^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [Qemu-devel] [PATCH 1/4] vnc: make the Buffer capacity increase in powers of two 2015-09-03 10:29 ` Peter Lieven @ 2015-09-03 10:47 ` Gerd Hoffmann 0 siblings, 0 replies; 20+ messages in thread From: Gerd Hoffmann @ 2015-09-03 10:47 UTC (permalink / raw) To: Peter Lieven; +Cc: qemu-devel On Do, 2015-09-03 at 12:29 +0200, Peter Lieven wrote: > - buffer->capacity = pow2ceil(buffer->capacity + len); > + buffer->capacity = pow2ceil(buffer->offset + len); Updated patch. thanks, Gerd ^ permalink raw reply [flat|nested] 20+ messages in thread
* [Qemu-devel] [PATCH 2/4] vnc: allow the Buffer to shrink again 2015-08-27 10:18 [Qemu-devel] [PATCH 0/4] VNC server memory savings Peter Lieven 2015-08-27 10:18 ` [Qemu-devel] [PATCH 1/4] vnc: make the Buffer capacity increase in powers of two Peter Lieven @ 2015-08-27 10:18 ` Peter Lieven 2015-08-27 10:39 ` Daniel P. Berrange 2015-08-27 10:18 ` [Qemu-devel] [PATCH 3/4] vnc-jobs: move buffer_reset to vnc_async_encoding_end Peter Lieven 2015-08-27 10:18 ` [Qemu-devel] [PATCH 4/4] vnc: destroy server surface if no client is connected Peter Lieven 3 siblings, 1 reply; 20+ messages in thread From: Peter Lieven @ 2015-08-27 10:18 UTC (permalink / raw) To: qemu-devel; +Cc: Peter Lieven, kraxel 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. Signed-off-by: Peter Lieven <pl@kamp.de> --- ui/vnc.c | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/ui/vnc.c b/ui/vnc.c index 8cfd2d8..061e337 100644 --- a/ui/vnc.c +++ b/ui/vnc.c @@ -654,6 +654,7 @@ void vnc_framebuffer_update(VncState *vs, int x, int y, int w, int h, } #define BUFFER_MIN_INIT_SIZE 4096 +#define BUFFER_MIN_SHRINK_SIZE 65536 void buffer_reserve(Buffer *buffer, size_t len) { @@ -674,9 +675,19 @@ uint8_t *buffer_end(Buffer *buffer) return buffer->buffer + buffer->offset; } +static void buffer_shrink(Buffer *buffer) { + size_t new = MAX(pow2ceil(buffer->offset) * 2, + BUFFER_MIN_SHRINK_SIZE); + if (new < buffer->capacity) { + buffer->buffer = g_realloc(buffer->buffer, new); + buffer->capacity = new; + } +} + void buffer_reset(Buffer *buffer) { - buffer->offset = 0; + buffer->offset = 0; + buffer_shrink(buffer); } void buffer_free(Buffer *buffer) @@ -698,6 +709,7 @@ void buffer_advance(Buffer *buf, size_t len) memmove(buf->buffer, buf->buffer + len, (buf->offset - len)); buf->offset -= len; + buffer_shrink(buf); } static void vnc_desktop_resize(VncState *vs) -- 1.9.1 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [Qemu-devel] [PATCH 2/4] vnc: allow the Buffer to shrink again 2015-08-27 10:18 ` [Qemu-devel] [PATCH 2/4] vnc: allow the Buffer to shrink again Peter Lieven @ 2015-08-27 10:39 ` Daniel P. Berrange 2015-08-27 12:36 ` Peter Lieven 2015-09-03 9:52 ` Gerd Hoffmann 0 siblings, 2 replies; 20+ messages in thread From: Daniel P. Berrange @ 2015-08-27 10:39 UTC (permalink / raw) To: Peter Lieven; +Cc: qemu-devel, kraxel 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. Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Qemu-devel] [PATCH 2/4] vnc: allow the Buffer to shrink again 2015-08-27 10:39 ` Daniel P. Berrange @ 2015-08-27 12:36 ` Peter Lieven 2015-09-03 9:52 ` Gerd Hoffmann 1 sibling, 0 replies; 20+ messages in thread From: Peter Lieven @ 2015-08-27 12:36 UTC (permalink / raw) To: Daniel P. Berrange; +Cc: qemu-devel, kraxel Am 27.08.2015 um 12:39 schrieb Daniel P. Berrange: > 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. Maybe my commit message was a bit unclear. The memory usage of all buffers went up to 100MB. The issue with the Buffer is that its not shrinking and that the date is effectivly in 3 Buffers before its transferred on the wire. Plus depending on the encoding used there are even more internal buffers. Peter ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Qemu-devel] [PATCH 2/4] vnc: allow the Buffer to shrink again 2015-08-27 10:39 ` Daniel P. Berrange 2015-08-27 12:36 ` Peter Lieven @ 2015-09-03 9:52 ` Gerd Hoffmann 2015-09-03 10:07 ` Peter Lieven 2015-09-03 10:36 ` Peter Lieven 1 sibling, 2 replies; 20+ messages in thread From: Gerd Hoffmann @ 2015-09-03 9:52 UTC (permalink / raw) To: Daniel P. Berrange; +Cc: Peter Lieven, qemu-devel [-- 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 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [Qemu-devel] [PATCH 2/4] vnc: allow the Buffer to shrink again 2015-09-03 9:52 ` Gerd Hoffmann @ 2015-09-03 10:07 ` Peter Lieven 2015-09-03 11:52 ` Gerd Hoffmann 2015-09-03 10:36 ` Peter Lieven 1 sibling, 1 reply; 20+ messages in thread From: Peter Lieven @ 2015-09-03 10:07 UTC (permalink / raw) To: Gerd Hoffmann, Daniel P. Berrange; +Cc: qemu-devel Am 03.09.2015 um 11:52 schrieb Gerd Hoffmann: > 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? I will try the patch out. Thank 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. We need a possibility to shrink the buffer after it has been used. Especially the queue->buffer. Thanks, Peter ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Qemu-devel] [PATCH 2/4] vnc: allow the Buffer to shrink again 2015-09-03 10:07 ` Peter Lieven @ 2015-09-03 11:52 ` Gerd Hoffmann 2015-09-03 14:52 ` Peter Lieven 0 siblings, 1 reply; 20+ messages in thread From: Gerd Hoffmann @ 2015-09-03 11:52 UTC (permalink / raw) To: Peter Lieven; +Cc: qemu-devel [-- 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 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [Qemu-devel] [PATCH 2/4] vnc: allow the Buffer to shrink again 2015-09-03 11:52 ` Gerd Hoffmann @ 2015-09-03 14:52 ` Peter Lieven 0 siblings, 0 replies; 20+ messages in thread From: Peter Lieven @ 2015-09-03 14:52 UTC (permalink / raw) To: Gerd Hoffmann; +Cc: qemu-devel 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 ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Qemu-devel] [PATCH 2/4] vnc: allow the Buffer to shrink again 2015-09-03 9:52 ` Gerd Hoffmann 2015-09-03 10:07 ` Peter Lieven @ 2015-09-03 10:36 ` Peter Lieven 2015-09-03 11:57 ` Gerd Hoffmann 1 sibling, 1 reply; 20+ messages in thread From: Peter Lieven @ 2015-09-03 10:36 UTC (permalink / raw) To: Gerd Hoffmann, Daniel P. Berrange; +Cc: qemu-devel Am 03.09.2015 um 11:52 schrieb Gerd Hoffmann: > 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? Simple case to create at least a reasonable allocation: -vga vmware and a client that prefers Hextile encoding. ~/git/qemu$ x86_64-softmmu/qemu-system-x86_64 -m 4096 -enable-kvm -cdrom ~/Downloads/ubuntu-14.04.3-desktop-amd64.iso -k de -usb -device usb-tablet -nodefaults -serial null -parallel null -cpu Westmere,enforce -vga vmware -monitor stdio -vnc :0 QEMU 2.4.50 monitor - type 'help' for more information (qemu) buffer_reserve: 11/out : 4 kB buffer_reserve: 11/in : 4 kB buffer_reserve: queue : 4 kB buffer_reserve: queue : 8 kB buffer_reserve: 11/jobs : 8 kB buffer_reserve: 11/out : 16 kB buffer_reserve: queue : 16 kB buffer_reserve: queue : 32 kB buffer_reserve: queue : 64 kB buffer_reserve: queue : 128 kB buffer_reserve: queue : 256 kB buffer_reserve: 11/jobs : 256 kB buffer_reserve: 11/out : 256 kB buffer_reserve: queue : 512 kB buffer_reserve: queue : 1024 kB buffer_reserve: queue : 2048 kB buffer_reserve: 11/jobs : 2048 kB buffer_reserve: 11/out : 2048 kB buffer_reserve: queue : 4096 kB buffer_reserve: queue : 8192 kB buffer_reserve: queue : 16384 kB buffer_reserve: 11/jobs : 16384 kB buffer_reserve: 11/out : 16384 kB Its not 100MB, but 50MB. Same with Tight encoding: (qemu) buffer_reserve: 11/out : 4 kB buffer_reserve: 11/in : 4 kB buffer_reserve: queue : 4 kB buffer_reserve: 11/t-tight: 4 kB buffer_reserve: 11/t-tight: 8 kB buffer_reserve: 11/t-tight: 16 kB buffer_reserve: 11/t-tight: 32 kB buffer_reserve: 11/t-tight: 64 kB buffer_reserve: 11/t-tight: 128 kB buffer_reserve: 11/t-tight: 256 kB buffer_reserve: 11/t-tight: 512 kB buffer_reserve: 11/t-zlib : 4 kB buffer_reserve: 11/jobs : 4 kB buffer_reserve: 11/t-zlib : 64 kB buffer_reserve: 11/t-tight: 1024 kB buffer_reserve: 11/t-tight: 2048 kB buffer_reserve: 11/t-tight: 4096 kB buffer_reserve: 11/t-tight: 8192 kB buffer_reserve: 11/t-tight: 16384 kB buffer_reserve: 11/t-zlib : 128 kB buffer_reserve: queue : 8 kB buffer_reserve: 11/jobs : 8 kB buffer_reserve: 11/out : 8 kB buffer_reserve: 11/t-zlib : 256 kB buffer_reserve: queue : 64 kB buffer_reserve: queue : 128 kB buffer_reserve: queue : 256 kB buffer_reserve: queue : 512 kB buffer_reserve: queue : 1024 kB buffer_reserve: queue : 2048 kB buffer_reserve: queue : 4096 kB buffer_reserve: queue : 8192 kB buffer_reserve: 11/jobs : 8192 kB buffer_reserve: 11/out : 8192 kB ~40MB. Peter ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Qemu-devel] [PATCH 2/4] vnc: allow the Buffer to shrink again 2015-09-03 10:36 ` Peter Lieven @ 2015-09-03 11:57 ` Gerd Hoffmann 2015-09-03 12:00 ` Peter Lieven 0 siblings, 1 reply; 20+ messages in thread From: Gerd Hoffmann @ 2015-09-03 11:57 UTC (permalink / raw) To: Peter Lieven; +Cc: qemu-devel Hi, > buffer_reserve: 11/t-tight: 4096 kB > buffer_reserve: 11/t-tight: 8192 kB > buffer_reserve: 11/t-tight: 16384 kB A bit more than what I see (4096 usually, 8192 once). No idea why. > buffer_reserve: queue : 8192 kB > buffer_reserve: 11/jobs : 8192 kB > buffer_reserve: 11/out : 8192 kB Noticeable more than what I see. Is this local or remote client? Maybe these stay smaller for me (local client) simply because qemu can put the data to the wire faster. cheers, Gerd ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Qemu-devel] [PATCH 2/4] vnc: allow the Buffer to shrink again 2015-09-03 11:57 ` Gerd Hoffmann @ 2015-09-03 12:00 ` Peter Lieven 0 siblings, 0 replies; 20+ messages in thread From: Peter Lieven @ 2015-09-03 12:00 UTC (permalink / raw) To: Gerd Hoffmann; +Cc: qemu-devel Am 03.09.2015 um 13:57 schrieb Gerd Hoffmann: > Hi, > >> buffer_reserve: 11/t-tight: 4096 kB >> buffer_reserve: 11/t-tight: 8192 kB >> buffer_reserve: 11/t-tight: 16384 kB > A bit more than what I see (4096 usually, 8192 once). No idea why. > >> buffer_reserve: queue : 8192 kB >> buffer_reserve: 11/jobs : 8192 kB >> buffer_reserve: 11/out : 8192 kB > Noticeable more than what I see. > > Is this local or remote client? Maybe these stay smaller for me (local > client) simply because qemu can put the data to the wire faster. Local. But maybe you don't have a fullscreen background image in the vserver? Peter ^ permalink raw reply [flat|nested] 20+ messages in thread
* [Qemu-devel] [PATCH 3/4] vnc-jobs: move buffer_reset to vnc_async_encoding_end 2015-08-27 10:18 [Qemu-devel] [PATCH 0/4] VNC server memory savings Peter Lieven 2015-08-27 10:18 ` [Qemu-devel] [PATCH 1/4] vnc: make the Buffer capacity increase in powers of two Peter Lieven 2015-08-27 10:18 ` [Qemu-devel] [PATCH 2/4] vnc: allow the Buffer to shrink again Peter Lieven @ 2015-08-27 10:18 ` Peter Lieven 2015-08-27 10:18 ` [Qemu-devel] [PATCH 4/4] vnc: destroy server surface if no client is connected Peter Lieven 3 siblings, 0 replies; 20+ messages in thread From: Peter Lieven @ 2015-08-27 10:18 UTC (permalink / raw) To: qemu-devel; +Cc: Peter Lieven, kraxel now that we are able to shrink the buffer it makes sense to move the buffer_reset to vnc_async_encoding_end to actually shrink the buffer when it is no longer used. Signed-off-by: Peter Lieven <pl@kamp.de> --- ui/vnc-jobs.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/ui/vnc-jobs.c b/ui/vnc-jobs.c index 22c9abc..201d9b7 100644 --- a/ui/vnc-jobs.c +++ b/ui/vnc-jobs.c @@ -195,8 +195,6 @@ static void vnc_async_encoding_start(VncState *orig, VncState *local) local->zrle = orig->zrle; local->output = queue->buffer; local->csock = -1; /* Don't do any network work on this thread */ - - buffer_reset(&local->output); } static void vnc_async_encoding_end(VncState *orig, VncState *local) @@ -208,6 +206,7 @@ static void vnc_async_encoding_end(VncState *orig, VncState *local) orig->lossy_rect = local->lossy_rect; queue->buffer = local->output; + buffer_reset(&queue->buffer); } static int vnc_worker_thread_loop(VncJobQueue *queue) -- 1.9.1 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* [Qemu-devel] [PATCH 4/4] vnc: destroy server surface if no client is connected 2015-08-27 10:18 [Qemu-devel] [PATCH 0/4] VNC server memory savings Peter Lieven ` (2 preceding siblings ...) 2015-08-27 10:18 ` [Qemu-devel] [PATCH 3/4] vnc-jobs: move buffer_reset to vnc_async_encoding_end Peter Lieven @ 2015-08-27 10:18 ` Peter Lieven 2015-09-03 9:57 ` Gerd Hoffmann 3 siblings, 1 reply; 20+ messages in thread From: Peter Lieven @ 2015-08-27 10:18 UTC (permalink / raw) To: qemu-devel; +Cc: Peter Lieven, kraxel if no client is connected there is no need to keep the server surface. Throw it away and replace it with a dummy surface to save memory. Signed-off-by: Peter Lieven <pl@kamp.de> --- ui/vnc.c | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/ui/vnc.c b/ui/vnc.c index 061e337..57f0e54 100644 --- a/ui/vnc.c +++ b/ui/vnc.c @@ -776,6 +776,11 @@ static void vnc_dpy_switch(DisplayChangeListener *dcl, vnc_abort_display_jobs(vd); + /* if no client is connected use a dummy surface */ + if (QTAILQ_EMPTY(&vd->clients)) { + surface = qemu_create_displaysurface(0, 0); + } + /* server surface */ qemu_pixman_image_unref(vd->server); vd->ds = surface; @@ -1263,6 +1268,9 @@ void vnc_disconnect_finish(VncState *vs) if (vs->initialized) { QTAILQ_REMOVE(&vs->vd->clients, vs, next); + if (QTAILQ_EMPTY(&vs->vd->clients)) { + vnc_dpy_switch(&vs->vd->dcl, NULL); + } qemu_remove_mouse_mode_change_notifier(&vs->mouse_mode_notifier); } @@ -3083,6 +3091,7 @@ void vnc_init_state(VncState *vs) { vs->initialized = true; VncDisplay *vd = vs->vd; + bool first_client; vs->last_x = -1; vs->last_y = -1; @@ -3095,9 +3104,15 @@ void vnc_init_state(VncState *vs) qemu_mutex_init(&vs->output_mutex); vs->bh = qemu_bh_new(vnc_jobs_bh, vs); + first_client = QTAILQ_EMPTY(&vd->clients); QTAILQ_INSERT_TAIL(&vd->clients, vs, next); - graphic_hw_update(vd->dcl.con); + if (first_client) { + /* set/restore the correct surface in the VNC server */ + console_select(0); + } else { + graphic_hw_update(vd->dcl.con); + } vnc_write(vs, "RFB 003.008\n", 12); vnc_flush(vs); -- 1.9.1 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [Qemu-devel] [PATCH 4/4] vnc: destroy server surface if no client is connected 2015-08-27 10:18 ` [Qemu-devel] [PATCH 4/4] vnc: destroy server surface if no client is connected Peter Lieven @ 2015-09-03 9:57 ` Gerd Hoffmann 2015-09-03 10:08 ` Peter Lieven 0 siblings, 1 reply; 20+ messages in thread From: Gerd Hoffmann @ 2015-09-03 9:57 UTC (permalink / raw) To: Peter Lieven; +Cc: qemu-devel On Do, 2015-08-27 at 12:18 +0200, Peter Lieven wrote: > if no client is connected there is no need to keep the server > surface. Throw it away and replace it with a dummy surface to > save memory. No dummy surface please. Just set vd->server = NULL. cheers, Gerd ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Qemu-devel] [PATCH 4/4] vnc: destroy server surface if no client is connected 2015-09-03 9:57 ` Gerd Hoffmann @ 2015-09-03 10:08 ` Peter Lieven 2015-09-03 10:54 ` Gerd Hoffmann 0 siblings, 1 reply; 20+ messages in thread From: Peter Lieven @ 2015-09-03 10:08 UTC (permalink / raw) To: Gerd Hoffmann; +Cc: qemu-devel Am 03.09.2015 um 11:57 schrieb Gerd Hoffmann: > On Do, 2015-08-27 at 12:18 +0200, Peter Lieven wrote: >> if no client is connected there is no need to keep the server >> surface. Throw it away and replace it with a dummy surface to >> save memory. > No dummy surface please. Just set vd->server = NULL. I can do that, but I have to check for vd->server == NULL at some points then. Peter ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Qemu-devel] [PATCH 4/4] vnc: destroy server surface if no client is connected 2015-09-03 10:08 ` Peter Lieven @ 2015-09-03 10:54 ` Gerd Hoffmann 0 siblings, 0 replies; 20+ messages in thread From: Gerd Hoffmann @ 2015-09-03 10:54 UTC (permalink / raw) To: Peter Lieven; +Cc: qemu-devel On Do, 2015-09-03 at 12:08 +0200, Peter Lieven wrote: > Am 03.09.2015 um 11:57 schrieb Gerd Hoffmann: > > On Do, 2015-08-27 at 12:18 +0200, Peter Lieven wrote: > >> if no client is connected there is no need to keep the server > >> surface. Throw it away and replace it with a dummy surface to > >> save memory. > > No dummy surface please. Just set vd->server = NULL. > > I can do that, but I have to check for vd->server == NULL at some points then. Sure. That'll shortcut code paths which should not have any effect anyway. You probably also want factor out server surface initialization into a function which is called for both first vnc connect and surface changes. Oh, and btw: in case the surface changes without resolution/depth changing (guest page flip) we might simply skip surface (re-)initialization. cheers, Gerd ^ permalink raw reply [flat|nested] 20+ messages in thread
end of thread, other threads:[~2015-09-03 14:53 UTC | newest] Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2015-08-27 10:18 [Qemu-devel] [PATCH 0/4] VNC server memory savings Peter Lieven 2015-08-27 10:18 ` [Qemu-devel] [PATCH 1/4] vnc: make the Buffer capacity increase in powers of two Peter Lieven 2015-09-03 8:56 ` Gerd Hoffmann 2015-09-03 10:29 ` Peter Lieven 2015-09-03 10:47 ` Gerd Hoffmann 2015-08-27 10:18 ` [Qemu-devel] [PATCH 2/4] vnc: allow the Buffer to shrink again Peter Lieven 2015-08-27 10:39 ` Daniel P. Berrange 2015-08-27 12:36 ` Peter Lieven 2015-09-03 9:52 ` Gerd Hoffmann 2015-09-03 10:07 ` Peter Lieven 2015-09-03 11:52 ` Gerd Hoffmann 2015-09-03 14:52 ` Peter Lieven 2015-09-03 10:36 ` Peter Lieven 2015-09-03 11:57 ` Gerd Hoffmann 2015-09-03 12:00 ` Peter Lieven 2015-08-27 10:18 ` [Qemu-devel] [PATCH 3/4] vnc-jobs: move buffer_reset to vnc_async_encoding_end Peter Lieven 2015-08-27 10:18 ` [Qemu-devel] [PATCH 4/4] vnc: destroy server surface if no client is connected Peter Lieven 2015-09-03 9:57 ` Gerd Hoffmann 2015-09-03 10:08 ` Peter Lieven 2015-09-03 10:54 ` Gerd Hoffmann
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.