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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



> 
> On Mon, Sep 04, 2017 at 10:50:44AM -0400, Frediano Ziglio wrote:
> > > 
> > > 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 ?
> 
> Hmm, is that "instead"? Or is it that the polling loop is blocking the
> default event loop/the main loop?
> 

Yes, entirely avoiding the event loop. Really, the code is

while (condition || timeout) {
  send()
  sleep() // really! no poll/select whatever
  receive()
}

I means is safe as you can't have recursive event loop call
but I don't like poll code that much.

> > 
> > > 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...
> 
> I would say this does not belong to this commit then, or you need an
> explanatory comment. I can see some head scratching from someone looking
> at git blame, and trying to understand theh link between what is said in
> the commit log, and the addition of  this _push..
> 
> Christophe
> 

I'll remove

Frediano
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel




[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]     [Monitors]