[Spice-devel] [spice-server PATCH] RFC: stream-device: do not reallocate dev->msg for each frame

[Frediano Ziglio]

> 
> In the past dev->msg was only allocated for cursor images, and
> after use dev->msg size was reduced.
> 
> Now, since commit dcc3f995d9f55, dev->msg is used for every frame.
> There is little point of calling reallocate with a large dev->msg size
> and then reallocate with a small size, for every frame.
> 
> This patch keeps only the "enlarge buffer when needed" part.
> 
> The drawback of this approach is that if a big buffer was used
> it is kept big (at least, in the near future), even if
> current frames are smaller.
> 
> Signed-off-by: Uri Lublin <uril at redhat.com>

Why RFC?

Looking at current code looks like for similar usage we use a static
"small" (some KB) buffer as most messages are small and do dynamic
alloc/free for every other message.

> ---
>  server/red-stream-device.c | 14 --------------
>  1 file changed, 14 deletions(-)
> 
> diff --git a/server/red-stream-device.c b/server/red-stream-device.c
> index d293dc1cc..eda6e6690 100644
> --- a/server/red-stream-device.c
> +++ b/server/red-stream-device.c
> @@ -175,14 +175,6 @@ stream_device_partial_read(StreamDevice *dev,
> SpiceCharDeviceInstance *sin)
>       * the next message */
>      if (handled) {
>          dev->hdr_pos = 0;
> -
> -        // Reallocate message buffer to the minimum.
> -        // Currently the only message that requires resizing is the cursor
> shape,
> -        // which is not expected to be sent so often.
> -        if (dev->msg_len > sizeof(*dev->msg)) {
> -            dev->msg = g_realloc(dev->msg, sizeof(*dev->msg));
> -            dev->msg_len = sizeof(*dev->msg);
> -        }
>      }
>  
>      if (handled || dev->has_error) {
> @@ -314,12 +306,6 @@ handle_msg_data(StreamDevice *dev,
> SpiceCharDeviceInstance *sin)
>      }
>  
>      /* make sure we have a large enough buffer for the whole frame */
> -    /* ---
> -     * TODO: Now that we copy partial data into the buffer, for each frame
> -     * the buffer is allocated and freed (look for g_realloc in
> -     * stream_device_partial_read).
> -     * Probably better to just keep the larger buffer.
> -     */
>      if (dev->msg_pos == 0) {
>          dev->frame_mmtime = reds_get_mm_time();
>          if (dev->msg_len < dev->hdr.size) {

Frediano