[Spice-devel] [PATCH spice-server v2 1/6] red-channel-client: Remove push call where not necessary

[Frediano Ziglio]

> 
> On Mon, Sep 04, 2017 at 12:02:05PM +0100, Frediano Ziglio wrote:
> > Now the push is done automatically when a PipeItem is added
> > (cfr commit 5c460de1a3972b7cf2b9b2944d0b500c3affc363
> > "worker: push data when clients can receive them"),
> > forcing a push cause only network fragmentation and is required
> > only if you are handling data in a loop instead of using the
> > default loop.
> 
> in a loop instead of using the default loop? Are you talking about

yes, confusing. In some ugly point of code there are some polling loop.
Basically instead of using events we try to send data, wait for a bit then
try to receive and so on. So

 forcing a push cause only network fragmentation and is required
 only if you are handling data in a polling loop instead of using the
 default event loop.

sounds better ?

> mainloops? I would assume mainloops, in which case I would phrase things
> as "is required only if you are handling data in the non-default
> mainloop (eg from a RedWorker thread)". But this does not explain why we
> need to add a _push to red_channel_client_push_ping(), is the timer it
> goes with only used by RedWorker?
> 

this is really paranoia... can save some micro seconds maybe...

> Christophe
> 

Frediano

> > 
> > Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
> > ---
> >  server/main-channel.c       | 4 ++--
> >  server/red-channel-client.c | 3 +--
> >  2 files changed, 3 insertions(+), 4 deletions(-)
> > 
> > diff --git a/server/main-channel.c b/server/main-channel.c
> > index c30374c3..e72e5d79 100644
> > --- a/server/main-channel.c
> > +++ b/server/main-channel.c
> > @@ -81,7 +81,7 @@ void main_channel_push_mouse_mode(MainChannel *main_chan,
> > SpiceMouseMode current
> >          .is_client_mouse_allowed=is_client_mouse_allowed,
> >      };
> >  
> > -    red_channel_pipes_new_add_push(RED_CHANNEL(main_chan),
> > +    red_channel_pipes_new_add(RED_CHANNEL(main_chan),
> >          main_mouse_mode_item_new, &info);
> >  }
> >  
> > @@ -140,7 +140,7 @@ void main_channel_push_multi_media_time(MainChannel
> > *main_chan, uint32_t time)
> >          .time = time,
> >      };
> >  
> > -    red_channel_pipes_new_add_push(RED_CHANNEL(main_chan),
> > +    red_channel_pipes_new_add(RED_CHANNEL(main_chan),
> >          main_multi_media_time_item_new, &info);
> >  }
> >  
> > diff --git a/server/red-channel-client.c b/server/red-channel-client.c
> > index 2612a6db..c45988b9 100644
> > --- a/server/red-channel-client.c
> > +++ b/server/red-channel-client.c
> > @@ -675,6 +675,7 @@ static void
> > red_channel_client_push_ping(RedChannelClient *rcc)
> >      rcc->priv->latency_monitor.id = rand();
> >      red_channel_client_pipe_add_type(rcc, RED_PIPE_ITEM_TYPE_PING);
> >      red_channel_client_pipe_add_type(rcc, RED_PIPE_ITEM_TYPE_PING);
> > +    red_channel_client_push(rcc);
> >  }
> >  
> >  static void red_channel_client_ping_timer(void *opaque)
> > @@ -1590,7 +1591,6 @@ void
> > red_channel_client_pipe_add_type(RedChannelClient *rcc, int
> > pipe_item_type)
> >  
> >      red_pipe_item_init(item, pipe_item_type);
> >      red_channel_client_pipe_add(rcc, item);
> > -    red_channel_client_push(rcc);
> >  }
> >  
> >  void red_channel_client_pipe_add_empty_msg(RedChannelClient *rcc, int
> >  msg_type)
> > @@ -1600,7 +1600,6 @@ void
> > red_channel_client_pipe_add_empty_msg(RedChannelClient *rcc, int msg_type)
> >      red_pipe_item_init(&item->base, RED_PIPE_ITEM_TYPE_EMPTY_MSG);
> >      item->msg = msg_type;
> >      red_channel_client_pipe_add(rcc, &item->base);
> > -    red_channel_client_push(rcc);
> >  }
> >  
> >  gboolean red_channel_client_pipe_is_empty(RedChannelClient *rcc)