[Spice-devel] RedPipeItem lifespan, past, present and ... bug

[Jonathon Jongsma]

On Thu, 2016-05-19 at 18:25 +0200, Christophe Fergeau wrote:
> Hey,
> 
> On Thu, May 19, 2016 at 06:14:52AM -0400, Frediano Ziglio wrote:
> > Possible future changes (please comment):
> > - would be good if red_channel_client_wait_pipe_item_sent could work even
> >   without the hold_item, one possible implementation is adding a fake
> > RedPipeItem
> >   which when removed will set a flag causing the loop to exit (better would
> > be
> >   to remove the function and implement this stuff in another way, there are
> > only
> >   a single call to make sure there are no pending drawing on a surface);
> 
> Yeah, I was wondering too if a fake pipe item could be used to be
> notified when the preceding pipe item has been sent.

With GObject, we could potentially add an "item-sent" signal that is emitted
whenever an item is sent. Then wait_pipe_item_sent() could run a loop while
handling this signal to and quit the loop when the appropriate item is sent? 

> 
> > - remove the RingItem and make possible to add the item to multiple clients,
> > this
> >   IMHO would make stuff much easier to understand;

yes.

> > - remove hold_item and use always red_pipe_item_ref;
> 
> Yup, I would do that, hold_item really looks like a convoluted _ref()
> used when not all pipe items had a refcount.

agreed

> 
> > - add another callback to RedPipeItem to send the item to make all send_item
> >   callbacks really small;
> 
> It probably makes sense too, maybe longer term. I don't think this
> is strictly related to the bug you discussed earlier though.

hmm, what would this new callback do? marshall the pipe item?

> 
> 
> > - remove atomic operation on RedPipeItem reference counter, I think was
> > added
> >   to make sure there was no thread issues but it's just slowing down big
> > servers
> >   with many cores.
> 
> Imo this would need to go with annotations saying in which thread the code
> is running (thinking about the display thread). Probably not a huge
> issue right now.
> 
> Christophe
> _______________________________________________
> Spice-devel mailing list
> Spice-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel