Re: [PATCH 8/9] worker: make sure we dispatch after releasing items

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

 



Hi

----- Original Message -----
> 
> 
> It seems that "watch" is related to watches on non-Dispatcher fds, and
> "dispatch" is related to the watch on the Dispatcher fd.
> 
> Items can be released in the "display" stage when we try to queue pipe items
> when the client is not connected (see red_channel_client_pipe_add(), which
> calls
> validate_pipe_add() which releases the pipe item if the client is not
> connected). They can also be released immediately after connection as part of
> dcc_start() -> dcc_push_surface_image(). In either of these cases, I don't
> think
> it's useful to trigger another source dispatch.
> 
> Those released in the "push" stage have just been sent to the client, so the
> only reason I can think to trigger another source dispatch here is perhaps to
> re
> -run the display/cursor handler immediately in the case that the client pipe
> item queue was full. But I have no idea whether this is a regular occurrence
> or
> not. I suspect this was not really the goal of this patch.

I actually think that was it. The worker thread may currently block because
the client is too slow. But then, when we release item, the pipe is going to
be smaller hopefully, and thus we can try to dispatch again without waiting
for the timeout. (waiting for timers can really degrade performances in some
cases, like you see today with the qemu virgl timer, but that's another
story where I can show you figures when we merge gl scanounts in spice ;)

> 
> The "dispatch" stage happens when we receive a message from the Dispatcher
> object. These messages from the Dispatcher can cause us to send new pipe
> items
> to the client, and they can also queue new items without sending them. For
> example, handle_dev_destroy_primary_surface() will flush all queued display
> and
> cursor commands (which results in a red_channel_push() to send those to the
> client, which will in turn release those pipe items), and then it does a
> bunch
> of stuff with dependent drawables (which may result in pipe items being
> released
> without being sent) and then it queues a DESTROY_SURFACE pipe item. It
> doesn't
> seem to me that the act of releasing any of those pipe items that I described
> above should result in an immediate source dispatch.
> 
> The "watch" stage is perhaps more interesting since it's the second highest
> total in your results. The watch handler is generally triggered when we
> receive
> data on the client socket. Different channels have different handlers, and
> different behaviors related to Pipe Items. Some handlers don't result in any
> messages sent back to the client (and thus, no pipe items are created or
> released). Some handlers result in a new pipe item being queued to be sent to
> the client (e.g. INPUTS_MOUSE_POSITION queues a MOUSE_MOTION_ACK item to send
> to
> the client), but do not send it directly. And some handlers send a message to
> the client immediately (SPICE_MSGC_ACK calls red_channel_client_push() and
> thus
> results in all of the sent items being released). Of these three different
> situations, only the third one (immediately sending a message to the client)
> would cause us to call release_item() and thus set the timeout to 0 and
> trigger
> another source dispatch. But I still don't understand why that would be
> useful.
> 
> So I'm left with a big question: What problem are we trying to solve here? I
> can't think of a problem that would be solved by this patch.
> 
> Jonathon
> _______________________________________________
> Spice-devel mailing list
> Spice-devel@xxxxxxxxxxxxxxxxxxxxx
> http://lists.freedesktop.org/mailman/listinfo/spice-devel
> 
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
http://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]