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

[Frediano Ziglio]

> 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.
> 

Rationale in another thread.
Yes, this patch will die young, I don't think this is a big issue.

Frediano

> > ---
> >  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
> >