[Spice-devel] [RFC v4 41/62] server/red_worker: start using SURFACES_FOREACH

[Marc-André Lureau]

(tbh, I don't understand all the implications of this patch, I have
only a partial view)

On Tue, Apr 26, 2011 at 12:55 PM, Alon Levy <alevy at redhat.com> wrote:
> ---
>  server/red_worker.c |   58 ++++++++++++++++++++++++++++++++++++--------------
>  1 files changed, 42 insertions(+), 16 deletions(-)
>
> diff --git a/server/red_worker.c b/server/red_worker.c
> index 1a85505..ebd9853 100644
> --- a/server/red_worker.c
> +++ b/server/red_worker.c
> @@ -3599,7 +3599,12 @@ static inline void red_process_drawable_surfaces(RedWorker *worker,
>
>  static inline void red_process_drawable(RedWorker *worker, RedDrawable *drawable, uint32_t group_id)
>  {
> -    red_process_drawable_surfaces(worker, &worker->surfaces, drawable, group_id);
> +    RingItem *link;
> +    Surfaces *surfaces;
> +
> +    SURFACES_FOREACH(link, surfaces, worker) {
> +        red_process_drawable_surfaces(worker, surfaces, drawable, group_id);
> +    }
>  }
>
>  static inline void red_create_surface(RedWorker *worker, Surfaces *surfaces,
> @@ -4325,7 +4330,12 @@ static void red_update_area_surfaces(RedWorker *worker, Surfaces *surfaces,
>
>  static void red_update_area(RedWorker *worker, const SpiceRect *area, int surface_id)
>  {
> -    red_update_area_surfaces(worker, &worker->surfaces, area, surface_id);
> +    Surfaces *surfaces;
> +    RingItem *link;
> +
> +    SURFACES_FOREACH(link, surfaces, worker) {
> +        red_update_area_surfaces(worker, surfaces, area, surface_id);
> +    }
>  }
>
>  #endif
> @@ -4603,25 +4613,41 @@ static int red_process_commands(RedWorker *worker, uint32_t max_pipe_size, int *
>
>  #define RED_RELEASE_BUNCH_SIZE 64
>
> -static void red_free_some(RedWorker *worker)
> +static void __red_free_some(RedWorker *worker, int *n)

We could use a better name, such as
red_worker_release_client_drawables(worker, RED_RELEASE_BUNCH_SIZE -
n);

I guess it should also release equally among the various clients
(instead of taking them in order). So that after the call, the various
clients have a more homogeneous amount.

>  {
> -    int n = 0;
> -    DisplayChannelClient *dcc = worker->surfaces.dcc;
> -    GlzSharedDictionary *glz_dict = dcc ? dcc->glz_dict : NULL;
> +    RingItem *link;
> +    Surfaces *surfaces;
>
> -    if (glz_dict) {
> -        // encoding using the dictionary is prevented since the following operations might
> -        // change the dictionary
> -        pthread_rwlock_wrlock(&glz_dict->encode_lock);
> -        n = red_display_free_some_independent_glz_drawables(dcc);
> +    SURFACES_FOREACH(link, surfaces, worker) {
> +        while (!ring_is_empty(&surfaces->current_list) && *n++ < RED_RELEASE_BUNCH_SIZE) {
> +            free_one_drawable(worker, surfaces, TRUE);
> +        }
>     }
> +}
>
> -    while (!ring_is_empty(&worker->surfaces.current_list) && n++ < RED_RELEASE_BUNCH_SIZE) {
> -        free_one_drawable(worker, &worker->surfaces, TRUE);
> -    }
> +static void red_free_some(RedWorker *worker)

Renaming here to would help (free some what?)

> +{
> +    int n = 0;
> +    RingItem *link;
> +    DisplayChannelClient *dcc;
> +    GlzSharedDictionary *glz_dict;
>
> -    if (glz_dict) {
> -        pthread_rwlock_unlock(&glz_dict->encode_lock);
> +    if (!worker->display_channel) {
> +        __red_free_some(worker, &n);
> +        return;
> +    }

Shouldn't we first release clients drawable, and after channel? Don't know...

> +    DCC_FOREACH(link, dcc, &worker->display_channel->common.base) {
> +        glz_dict = dcc->glz_dict;
> +        if (glz_dict) {
> +            // encoding using the dictionary is prevented since the following operations might
> +            // change the dictionary
> +            pthread_rwlock_wrlock(&glz_dict->encode_lock);
> +            n += red_display_free_some_independent_glz_drawables(dcc);
> +        }
> +        __red_free_some(worker, &n);
> +        if (glz_dict) {
> +            pthread_rwlock_unlock(&glz_dict->encode_lock);
> +        }
>     }
>  }
>
> --
> 1.7.4.4
>
> _______________________________________________
> Spice-devel mailing list
> Spice-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/spice-devel
>



-- 
Marc-André Lureau