[Spice-devel,v2,2/2] display-channel: Handle timeout for joining drawables

Submitted by Frediano Ziglio on March 2, 2017, 11:34 a.m.

Details

Message ID 476b865ff6903578c2e97d1895e17a2916984bc2.1488454452.git-series.fziglio@redhat.com
State New
Headers show
Series "RHEL7 improvements" ( rev: 3 ) in Spice

Not browsing as part of any series.

Commit Message

Frediano Ziglio March 2, 2017, 11:34 a.m.
The previous patch join correctly the commands however if there
are no more commands the command joined is delayed till new
commands arrive. This patch introduce a timeout (currently 10 ms)
after the command is executed.

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
---
 server/display-channel-private.h |  4 ++++-
 server/display-channel.c         | 36 ++++++++++++++++++++++++---------
 2 files changed, 31 insertions(+), 9 deletions(-)

Patch hide | download patch | download mbox

diff --git a/server/display-channel-private.h b/server/display-channel-private.h
index 62e03b6..e289e45 100644
--- a/server/display-channel-private.h
+++ b/server/display-channel-private.h
@@ -20,6 +20,8 @@ 
 
 #include "display-channel.h"
 
+#define JOINABLE_TIMEOUT 10 // ms
+
 #define NUM_DRAWABLES 1000
 typedef struct _Drawable _Drawable;
 struct _Drawable {
@@ -69,6 +71,8 @@  struct DisplayChannelPrivate
 
     int gl_draw_async_count;
 
+    uint32_t joinable_generation;
+    SpiceTimer *joinable_timeout;
     RedDrawable *joinable_drawable;
 
 /* TODO: some day unify this, make it more runtime.. */
diff --git a/server/display-channel.c b/server/display-channel.c
index 37d1703..8993b12 100644
--- a/server/display-channel.c
+++ b/server/display-channel.c
@@ -78,6 +78,9 @@  display_channel_finalize(GObject *object)
 {
     DisplayChannel *self = DISPLAY_CHANNEL(object);
 
+    SpiceCoreInterfaceInternal* core = red_channel_get_core_interface(RED_CHANNEL(self));
+    core->timer_remove(core, self->priv->joinable_timeout);
+
     display_channel_destroy_surfaces(self);
     image_cache_reset(&self->priv->image_cache);
     monitors_config_unref(self->priv->monitors_config);
@@ -1330,17 +1333,23 @@  display_channel_process_draw_single(DisplayChannel *display, RedDrawable *red_dr
     drawable_unref(drawable);
 }
 
+static void display_joinable_timeout(void *opaque)
+{
+    DisplayChannel *display = opaque;
+    if (display->priv->joinable_drawable) {
+        display_channel_process_draw_single(display, display->priv->joinable_drawable,
+                                            display->priv->joinable_generation);
+        red_drawable_unref(display->priv->joinable_drawable);
+        display->priv->joinable_drawable = NULL;
+    }
+}
+
 void display_channel_process_draw(DisplayChannel *display, RedDrawable *red_drawable,
                                   uint32_t process_commands_generation)
 {
     if (!red_drawable_joinable(red_drawable)) {
         // not joinable, process all
-        if (display->priv->joinable_drawable) {
-            display_channel_process_draw_single(display, display->priv->joinable_drawable,
-                                                process_commands_generation);
-            red_drawable_unref(display->priv->joinable_drawable);
-            display->priv->joinable_drawable = NULL;
-        }
+        display_joinable_timeout(display);
         display_channel_process_draw_single(display, red_drawable, process_commands_generation);
         return;
     }
@@ -1351,12 +1360,15 @@  void display_channel_process_draw(DisplayChannel *display, RedDrawable *red_draw
         red_drawable = red_drawable_join(display->priv->joinable_drawable, red_drawable);
     } else {
         // they can't be joined
-        display_channel_process_draw_single(display, display->priv->joinable_drawable,
-                                            process_commands_generation);
-        red_drawable_unref(display->priv->joinable_drawable);
+        display_joinable_timeout(display);
     }
+
     // try to join with next one
     red_drawable_ref(red_drawable);
+
+    SpiceCoreInterfaceInternal* core = red_channel_get_core_interface(RED_CHANNEL(display));
+    core->timer_start(core, display->priv->joinable_timeout, JOINABLE_TIMEOUT);
+    display->priv->joinable_generation = process_commands_generation;
     display->priv->joinable_drawable = red_drawable;
 }
 
@@ -2227,6 +2239,12 @@  display_channel_constructed(GObject *object)
     self->priv->stream_video = SPICE_STREAM_VIDEO_OFF;
     display_channel_init_streams(self);
 
+    SpiceCoreInterfaceInternal* core = red_channel_get_core_interface(channel);
+    self->priv->joinable_timeout = core->timer_add(core, display_joinable_timeout, self);
+    if (!self->priv->joinable_timeout) {
+        spice_error("joinable timer create failed");
+    }
+
     red_channel_set_cap(channel, SPICE_DISPLAY_CAP_MONITORS_CONFIG);
     red_channel_set_cap(channel, SPICE_DISPLAY_CAP_PREF_COMPRESSION);
     red_channel_set_cap(channel, SPICE_DISPLAY_CAP_STREAM_REPORT);

Comments

I'm trying to decide if there's any benefit to splitting this out from
the other patch. I'm leaning toward squashing them.

On Thu, 2017-03-02 at 11:34 +0000, Frediano Ziglio wrote:
> The previous patch join correctly the commands however if there
> are no more commands the command joined is delayed till new
> commands arrive. This patch introduce a timeout (currently 10 ms)
> after the command is executed.
> 
> Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
> ---
>  server/display-channel-private.h |  4 ++++-
>  server/display-channel.c         | 36 ++++++++++++++++++++++++----
> -----
>  2 files changed, 31 insertions(+), 9 deletions(-)
> 
> diff --git a/server/display-channel-private.h b/server/display-
> channel-private.h
> index 62e03b6..e289e45 100644
> --- a/server/display-channel-private.h
> +++ b/server/display-channel-private.h
> @@ -20,6 +20,8 @@
>  
>  #include "display-channel.h"
>  
> +#define JOINABLE_TIMEOUT 10 // ms
> +
>  #define NUM_DRAWABLES 1000
>  typedef struct _Drawable _Drawable;
>  struct _Drawable {
> @@ -69,6 +71,8 @@ struct DisplayChannelPrivate
>  
>      int gl_draw_async_count;
>  
> +    uint32_t joinable_generation;
> +    SpiceTimer *joinable_timeout;
>      RedDrawable *joinable_drawable;
>  
>  /* TODO: some day unify this, make it more runtime.. */
> diff --git a/server/display-channel.c b/server/display-channel.c
> index 37d1703..8993b12 100644
> --- a/server/display-channel.c
> +++ b/server/display-channel.c
> @@ -78,6 +78,9 @@ display_channel_finalize(GObject *object)
>  {
>      DisplayChannel *self = DISPLAY_CHANNEL(object);
>  
> +    SpiceCoreInterfaceInternal* core =
> red_channel_get_core_interface(RED_CHANNEL(self));
> +    core->timer_remove(core, self->priv->joinable_timeout);
> +
>      display_channel_destroy_surfaces(self);
>      image_cache_reset(&self->priv->image_cache);
>      monitors_config_unref(self->priv->monitors_config);
> @@ -1330,17 +1333,23 @@
> display_channel_process_draw_single(DisplayChannel *display,
> RedDrawable *red_dr
>      drawable_unref(drawable);
>  }
>  
> +static void display_joinable_timeout(void *opaque)
> +{
> +    DisplayChannel *display = opaque;
> +    if (display->priv->joinable_drawable) {
> +        display_channel_process_draw_single(display, display->priv-
> >joinable_drawable,
> +                                            display->priv-
> >joinable_generation);
> +        red_drawable_unref(display->priv->joinable_drawable);
> +        display->priv->joinable_drawable = NULL;
> +    }
> +}
> +
>  void display_channel_process_draw(DisplayChannel *display,
> RedDrawable *red_drawable,
>                                    uint32_t
> process_commands_generation)
>  {
>      if (!red_drawable_joinable(red_drawable)) {
>          // not joinable, process all
> -        if (display->priv->joinable_drawable) {
> -            display_channel_process_draw_single(display, display-
> >priv->joinable_drawable,
> -                                                process_commands_gen
> eration);
> -            red_drawable_unref(display->priv->joinable_drawable);
> -            display->priv->joinable_drawable = NULL;
> -        }
> +        display_joinable_timeout(display);

Here I see that my comment on the previous patch (about
process_commands_generation) is no longer an issue because it's now
saved and used in display_joinable_timeout()

>          display_channel_process_draw_single(display, red_drawable,
> process_commands_generation);
>          return;
>      }
> @@ -1351,12 +1360,15 @@ void
> display_channel_process_draw(DisplayChannel *display, RedDrawable
> *red_draw
>          red_drawable = red_drawable_join(display->priv-
> >joinable_drawable, red_drawable);
>      } else {
>          // they can't be joined
> -        display_channel_process_draw_single(display, display->priv-
> >joinable_drawable,
> -                                            process_commands_generat
> ion);
> -        red_drawable_unref(display->priv->joinable_drawable);
> +        display_joinable_timeout(display);
>      }
> +
>      // try to join with next one
>      red_drawable_ref(red_drawable);
> +
> +    SpiceCoreInterfaceInternal* core =
> red_channel_get_core_interface(RED_CHANNEL(display));
> +    core->timer_start(core, display->priv->joinable_timeout,
> JOINABLE_TIMEOUT);
> +    display->priv->joinable_generation =
> process_commands_generation;
>      display->priv->joinable_drawable = red_drawable;
>  }
>  
> @@ -2227,6 +2239,12 @@ display_channel_constructed(GObject *object)
>      self->priv->stream_video = SPICE_STREAM_VIDEO_OFF;
>      display_channel_init_streams(self);
>  
> +    SpiceCoreInterfaceInternal* core =
> red_channel_get_core_interface(channel);
> +    self->priv->joinable_timeout = core->timer_add(core,
> display_joinable_timeout, self);
> +    if (!self->priv->joinable_timeout) {
> +        spice_error("joinable timer create failed");
> +    }
> +
>      red_channel_set_cap(channel, SPICE_DISPLAY_CAP_MONITORS_CONFIG);
>      red_channel_set_cap(channel,
> SPICE_DISPLAY_CAP_PREF_COMPRESSION);
>      red_channel_set_cap(channel, SPICE_DISPLAY_CAP_STREAM_REPORT);


Aside from the comment about squashing, I think this one looks fine

Acked-by: Jonathon Jongsma <jjongsma@redhat.com>