[Spice-devel] [PATCH] timer: use TLS to store and set current timer queue

[Marc-André Lureau]

Hi

----- Original Message -----
> Avoid locking, list and complicated stuff and use TLS.
> 
> Signed-off-by: Frediano Ziglio <fziglio at redhat.com>

That's all dropped with the GMainLoop patch, is it worth improving this code?

What's the rationale of postponing the loop patch? Too risky? compared to the rest of the changes, maybe not imho.

> ---
>  server/spice_timer_queue.c | 63
>  ++++++----------------------------------------
>  1 file changed, 8 insertions(+), 55 deletions(-)
> 
> diff --git a/server/spice_timer_queue.c b/server/spice_timer_queue.c
> index 60017cc..abab878 100644
> --- a/server/spice_timer_queue.c
> +++ b/server/spice_timer_queue.c
> @@ -21,15 +21,6 @@
>  #include "spice_timer_queue.h"
>  #include "common/ring.h"
>  
> -static Ring timer_queue_list;
> -static int queue_count = 0;
> -static pthread_mutex_t queue_list_lock = PTHREAD_MUTEX_INITIALIZER;
> -
> -static void spice_timer_queue_init(void)
> -{
> -    ring_init(&timer_queue_list);
> -}
> -
>  struct SpiceTimer {
>      RingItem link;
>      RingItem active_link;
> @@ -45,48 +36,18 @@ struct SpiceTimer {
>  };
>  
>  struct SpiceTimerQueue {
> -    RingItem link;
>      pthread_t thread;
>      Ring timers;
>      Ring active_timers;
>  };
>  
> -static SpiceTimerQueue *spice_timer_queue_find(void)
> -{
> -    pthread_t self = pthread_self();
> -    RingItem *queue_item;
> -
> -    RING_FOREACH(queue_item, &timer_queue_list) {
> -         SpiceTimerQueue *queue = SPICE_CONTAINEROF(queue_item,
> SpiceTimerQueue, link);
> -
> -         if (pthread_equal(self, queue->thread) != 0) {
> -            return queue;
> -         }
> -    }
> -
> -    return NULL;
> -}
> -
> -static SpiceTimerQueue *spice_timer_queue_find_with_lock(void)
> -{
> -    SpiceTimerQueue *queue;
> -
> -    pthread_mutex_lock(&queue_list_lock);
> -    queue = spice_timer_queue_find();
> -    pthread_mutex_unlock(&queue_list_lock);
> -    return queue;
> -}
> +static __thread SpiceTimerQueue *current_queue = NULL;
>  
>  int spice_timer_queue_create(void)
>  {
>      SpiceTimerQueue *queue;
>  
> -    pthread_mutex_lock(&queue_list_lock);
> -    if (queue_count == 0) {
> -        spice_timer_queue_init();
> -    }
> -
> -    if (spice_timer_queue_find() != NULL) {
> +    if (current_queue != NULL) {
>          spice_printerr("timer queue was already created for the thread");
>          return FALSE;
>      }
> @@ -96,10 +57,7 @@ int spice_timer_queue_create(void)
>      ring_init(&queue->timers);
>      ring_init(&queue->active_timers);
>  
> -    ring_add(&timer_queue_list, &queue->link);
> -    queue_count++;
> -
> -    pthread_mutex_unlock(&queue_list_lock);
> +    current_queue = queue;
>  
>      return TRUE;
>  }
> @@ -107,10 +65,9 @@ int spice_timer_queue_create(void)
>  void spice_timer_queue_destroy(void)
>  {
>      RingItem *item;
> -    SpiceTimerQueue *queue;
> +    SpiceTimerQueue *queue = current_queue;
>  
> -    pthread_mutex_lock(&queue_list_lock);
> -    queue = spice_timer_queue_find();
> +    current_queue = NULL;
>  
>      spice_assert(queue != NULL);
>  
> @@ -121,17 +78,13 @@ void spice_timer_queue_destroy(void)
>          spice_timer_remove(timer);
>      }
>  
> -    ring_remove(&queue->link);
>      free(queue);
> -    queue_count--;
> -
> -    pthread_mutex_unlock(&queue_list_lock);
>  }
>  
>  SpiceTimer *spice_timer_queue_add(SpiceTimerFunc func, void *opaque)
>  {
>      SpiceTimer *timer = spice_new0(SpiceTimer, 1);
> -    SpiceTimerQueue *queue = spice_timer_queue_find_with_lock();
> +    SpiceTimerQueue *queue = current_queue;
>  
>      spice_assert(queue != NULL);
>  
> @@ -221,7 +174,7 @@ unsigned int spice_timer_queue_get_timeout_ms(void)
>      int64_t now_ms;
>      RingItem *head;
>      SpiceTimer *head_timer;
> -    SpiceTimerQueue *queue = spice_timer_queue_find_with_lock();
> +    SpiceTimerQueue *queue = current_queue;
>  
>      spice_assert(queue != NULL);
>  
> @@ -244,7 +197,7 @@ void spice_timer_queue_cb(void)
>      struct timespec now;
>      uint64_t now_ms;
>      RingItem *head;
> -    SpiceTimerQueue *queue = spice_timer_queue_find_with_lock();
> +    SpiceTimerQueue *queue = current_queue;
>  
>      spice_assert(queue != NULL);
>  
> --
> 2.4.3
> 
> _______________________________________________
> Spice-devel mailing list
> Spice-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/spice-devel
>