[Spice-devel] [spice-gtk v3] vmcstream, gtask: Do idle ourself instead of leaving it to GTask's heuristic

[Fabiano Fidêncio]

Pushed, thanks!

On Tue, Mar 22, 2016 at 11:01 PM, Fabiano Fidêncio <fabiano at fidencio.org> wrote:
> On Tue, Mar 22, 2016 at 10:57 PM, Marc-André Lureau <mlureau at redhat.com> wrote:
>> ack
>>
>> ----- Original Message -----
>>> Seems that GTask heuristic only makes sense in a non-coroutine case.
>>> After opening a bug[0] on spice-gtk and a few discussions in the mailing
>>> list, seems that is completely fine for coroutine code to deal with the
>>> idle explicitly.
>>>
>>> Signed-off-by: Fabiano Fidêncio <fidencio at redhat.com>
>>> ---
>>> Changes since v2, as per Marc-André review:
>>> - Change the commit log
>>> - Change the comment in the code
>>> - No need for a _free() function, just do the unref and free inside
>>> _idle_cb()
>>>
>>> Changes since v1, as per Marc-André review:
>>> - Use g_idle_add() instead of GSource ... which worries me a bit about the
>>>   context used when using g_idle_add(), as previously the context used was
>>>   the one from the GTask.
>>> ---
>>>
>>>  src/vmcstream.c | 29 +++++++++++++++++++++++++++--
>>>  1 file changed, 27 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/src/vmcstream.c b/src/vmcstream.c
>>> index 5ebf15a..5dd2799 100644
>>> --- a/src/vmcstream.c
>>> +++ b/src/vmcstream.c
>>> @@ -97,6 +97,24 @@ spice_vmc_input_stream_new(void)
>>>      return self;
>>>  }
>>>
>>> +typedef struct _complete_in_idle_cb_data {
>>> +    GTask *task;
>>> +    gssize pos;
>>> +} complete_in_idle_cb_data;
>>> +
>>> +static gboolean
>>> +complete_in_idle_cb(gpointer user_data)
>>> +{
>>> +    complete_in_idle_cb_data *data = user_data;
>>> +
>>> +    g_task_return_int(data->task, data->pos);
>>> +
>>> +    g_object_unref (data->task);
>>> +    g_free (data);
>>> +
>>> +    return FALSE;
>>> +}
>>> +
>>>  /* coroutine */
>>>  /**
>>>   * Feed a SpiceVmc stream with new data from a coroutine
>>> @@ -116,6 +134,8 @@ spice_vmc_input_stream_co_data(SpiceVmcInputStream *self,
>>>      self->coroutine = coroutine_self();
>>>
>>>      while (size > 0) {
>>> +        complete_in_idle_cb_data *cb_data;
>>> +
>>>          SPICE_DEBUG("spicevmc co_data %p", self->task);
>>>          if (!self->task)
>>>              coroutine_yield(NULL);
>>> @@ -137,10 +157,15 @@ spice_vmc_input_stream_co_data(SpiceVmcInputStream
>>> *self,
>>>          if (self->all && min > 0 && self->pos != self->count)
>>>              continue;
>>>
>>> -        g_task_return_int(self->task, self->pos);
>>> +        /* Let's deal with the task complete in idle by ourselves, as GTask
>>> +         * heuristic only makes sense in a non-coroutine case.
>>> +         */
>>> +        cb_data = g_new(complete_in_idle_cb_data , 1);
>>> +        cb_data->task = g_object_ref(self->task);
>>> +        cb_data->pos = self->pos;
>>> +        g_idle_add(complete_in_idle_cb, cb_data);
>>>
>>>          g_cancellable_disconnect(g_task_get_cancellable(self->task),
>>>          self->cancel_id);
>>> -
>>>          g_clear_object(&self->task);
>>>      }
>>>
>>> --
>>> 2.5.0
>>>
>>> _______________________________________________
>>> Spice-devel mailing list
>>> Spice-devel at lists.freedesktop.org
>>> https://lists.freedesktop.org/mailman/listinfo/spice-devel
>>>
>> _______________________________________________
>> Spice-devel mailing list
>> Spice-devel at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/spice-devel
>
>
>
> --
> Fabiano Fidêncio



-- 
Fabiano Fidêncio