[Spice-devel] [PATCH spice-gtk 08/14] channel: do not reenter the mainloop at every iteration
Marc-André Lureau
marcandre.lureau at gmail.com
Tue Sep 10 14:20:12 PDT 2013
argh, I reintroduced it by mistake..
On Tue, Sep 10, 2013 at 9:15 PM, Yonit Halperin <yhalperi at redhat.com> wrote:
> Hi,
>
>
> On 09/10/2013 10:44 AM, Marc-André Lureau wrote:
>>
>> From: Marc-André Lureau <marcandre.lureau at gmail.com>
>>
>> The current coroutine channel_iterate() creates a GSource for the socket
>> reenters the mainloop waiting for IO condition. This is really heavy
>> usage of mainloop showing up in system time and user space
>> profiling (10% of CPU time spent in mainloop overhead). Instead flush
>> all pending input messages before switching context and reentering main
>> loop.
>> ---
>> gtk/spice-channel.c | 57
>> ++++++++++++++++++++++++-----------------------------
>> 1 file changed, 26 insertions(+), 31 deletions(-)
>>
>> diff --git a/gtk/spice-channel.c b/gtk/spice-channel.c
>> index 46caeef..01598d3 100644
>> --- a/gtk/spice-channel.c
>> +++ b/gtk/spice-channel.c
>> @@ -2080,10 +2080,22 @@ static void
>> spice_channel_iterate_read(SpiceChannel *channel)
>> {
>> SpiceChannelPrivate *c = channel->priv;
>>
>> - g_return_if_fail(c->state != SPICE_CHANNEL_STATE_MIGRATING);
>> + /* treat all incoming data (block on message completion) */
>> + while (!c->has_error &&
>> + c->state != SPICE_CHANNEL_STATE_MIGRATING &&
>> + g_coroutine_socket_wait(&c->coroutine, c->sock, G_IO_IN) &
>> G_IO_IN) {
>> +
>
> This is logically different from the previous version of the patch. Didn't
> you want to use g_socket_condition_check here, and call
> g_coroutine_socket_wait in between the calls to iterate_write and
> iterate_read in order to save some unnecessary switches to the main loop?
> Besides that and and the nitpick for patch #3, the series looks good. Ack.
>
>> + do
>> + spice_channel_recv_msg(channel,
>> +
>> (handler_msg_in)SPICE_CHANNEL_GET_CLASS(channel)->handle_msg, NULL);
>> +#if HAVE_SASL
>> + /* flush the sasl buffer too */
>> + while (c->sasl_decoded != NULL);
>> +#else
>> + while (FALSE);
>> +#endif
>> + }
>>
>> - spice_channel_recv_msg(channel,
>> - (handler_msg_in)SPICE_CHANNEL_GET_CLASS(channel)->handle_msg,
>> NULL);
>> }
>>
>> static gboolean wait_migration(gpointer data)
>> @@ -2105,37 +2117,20 @@ static gboolean spice_channel_iterate(SpiceChannel
>> *channel)
>> SpiceChannelPrivate *c = channel->priv;
>> GIOCondition ret;
>>
>> - do {
>> - if (c->state == SPICE_CHANNEL_STATE_MIGRATING &&
>> - !g_coroutine_condition_wait(&c->coroutine, wait_migration,
>> channel))
>> - CHANNEL_DEBUG(channel, "migration wait cancelled");
>> -
>> - if (c->has_error) {
>> - CHANNEL_DEBUG(channel, "channel has error, breaking loop");
>> - return FALSE;
>> - }
>> -
>> - SPICE_CHANNEL_GET_CLASS(channel)->iterate_write(channel);
>> + if (c->state == SPICE_CHANNEL_STATE_MIGRATING &&
>> + !g_coroutine_condition_wait(&c->coroutine, wait_migration,
>> channel))
>> + CHANNEL_DEBUG(channel, "migration wait cancelled");
>>
>> - ret = g_coroutine_socket_wait(&c->coroutine, c->sock,
>> - c->state != SPICE_CHANNEL_STATE_MIGRATING ? G_IO_IN : 0);
>> -
>> -#ifdef WIN32
>> - /* FIXME: windows gsocket is buggy, it doesn't return correct
>> condition... */
>> - ret = g_socket_condition_check(c->sock, G_IO_IN);
>> -#endif
>> - } while (ret == 0); /* ret == 0 means no IO condition, but woken up
>> */
>> -
>> - if (ret & G_IO_IN) {
>> - do
>> - SPICE_CHANNEL_GET_CLASS(channel)->iterate_read(channel);
>> -#if HAVE_SASL
>> - while (c->sasl_decoded != NULL);
>> -#else
>> - while (FALSE);
>> -#endif
>> + if (c->has_error) {
>> + CHANNEL_DEBUG(channel, "channel has error, breaking loop");
>> + return FALSE;
>> }
>>
>> + /* flush any pending write and read */
>> + SPICE_CHANNEL_GET_CLASS(channel)->iterate_write(channel);
>> + SPICE_CHANNEL_GET_CLASS(channel)->iterate_read(channel);
>> +
>> + ret = g_socket_condition_check(c->sock, G_IO_IN | G_IO_ERR |
>> G_IO_HUP);
>> if (c->state > SPICE_CHANNEL_STATE_CONNECTING &&
>> ret & (G_IO_ERR|G_IO_HUP)) {
>> SPICE_DEBUG("got socket error: %d", ret);
>>
>
--
Marc-André Lureau
More information about the Spice-devel
mailing list