[Spice-devel] [PATCH 09/11] utils: add red_get_monotonic_time()

[Jonathon Jongsma]

On Thu, 2015-10-29 at 07:50 -0400, Frediano Ziglio wrote:
> > 
> > From: Marc-André Lureau <marcandre.lureau at gmail.com>
> > 
> > ---
> >  server/Makefile.am  |  1 +
> >  server/red_worker.c | 18 ++++--------------
> >  server/utils.h      | 32 ++++++++++++++++++++++++++++++++
> >  3 files changed, 37 insertions(+), 14 deletions(-)
> >  create mode 100644 server/utils.h
> > 
> > diff --git a/server/Makefile.am b/server/Makefile.am
> > index 522e926..28757ab 100644
> > --- a/server/Makefile.am
> > +++ b/server/Makefile.am
> > @@ -134,6 +134,7 @@ libspice_server_la_SOURCES =			
> > \
> >  	pixmap-cache.c				\
> >  	tree.h				\
> >  	tree.c				\
> > +	utils.h					\
> >  	$(NULL)
> >  
> >  if HAVE_GL
> > diff --git a/server/red_worker.c b/server/red_worker.c
> > index ab8b22f..9ccda1d 100644
> > --- a/server/red_worker.c
> > +++ b/server/red_worker.c
> > @@ -65,6 +65,7 @@
> >  #include "spice_timer_queue.h"
> >  #include "cursor-channel.h"
> >  #include "tree.h"
> > +#include "utils.h"
> >  
> >  //#define COMPRESS_STAT
> >  //#define DUMP_BITMAP
> > @@ -125,11 +126,6 @@ static void rendering_incorrect(const char
> > *msg)
> >      spice_warning("rendering incorrect from now on: %s", msg);
> >  }
> >  
> > -static inline red_time_t timespec_to_red_time(struct timespec
> > *time)
> > -{
> > -    return (red_time_t) time->tv_sec * (1000 * 1000 * 1000) + time
> > ->tv_nsec;
> > -}
> > -
> >  typedef unsigned long stat_time_t;
> >  
> >  #if defined(RED_WORKER_STAT) || defined(COMPRESS_STAT)
> > @@ -2160,10 +2156,8 @@ static inline unsigned int
> > red_get_streams_timout(RedWorker *worker)
> >      unsigned int timout = -1;
> >      Ring *ring = &worker->streams;
> >      RingItem *item = ring;
> > -    struct timespec time;
> >  
> > -    clock_gettime(CLOCK_MONOTONIC, &time);
> > -    red_time_t now = timespec_to_red_time(&time);
> > +    red_time_t now = red_get_monotonic_time();
> >      while ((item = ring_next(ring, item))) {
> >          Stream *stream;
> >  
> > @@ -2181,11 +2175,9 @@ static inline unsigned int
> > red_get_streams_timout(RedWorker *worker)
> >  static inline void red_handle_streams_timout(RedWorker *worker)
> >  {
> >      Ring *ring = &worker->streams;
> > -    struct timespec time;
> >      RingItem *item;
> >  
> > -    clock_gettime(CLOCK_MONOTONIC, &time);
> > -    red_time_t now = timespec_to_red_time(&time);
> > +    red_time_t now = red_get_monotonic_time();
> >      item = ring_get_head(ring);
> >      while (item) {
> >          Stream *stream = SPICE_CONTAINEROF(item, Stream, link);
> > @@ -3328,7 +3320,6 @@ static Drawable *get_drawable(RedWorker
> > *worker,
> > uint8_t effect, RedDrawable *re
> >                                uint32_t group_id)
> >  {
> >      Drawable *drawable;
> > -    struct timespec time;
> >      int x;
> >  
> >      VALIDATE_SURFACE_RETVAL(worker, red_drawable->surface_id,
> > NULL)
> > @@ -3348,8 +3339,7 @@ static Drawable *get_drawable(RedWorker
> > *worker,
> > uint8_t effect, RedDrawable *re
> >      worker->drawable_count++;
> >      memset(drawable, 0, sizeof(Drawable));
> >      drawable->refs = 1;
> > -    clock_gettime(CLOCK_MONOTONIC, &time);
> > -    drawable->creation_time = timespec_to_red_time(&time);
> > +    drawable->creation_time = red_get_monotonic_time();
> >      ring_item_init(&drawable->list_link);
> >      ring_item_init(&drawable->surface_list_link);
> >      ring_item_init(&drawable->tree_item.base.siblings_link);
> > diff --git a/server/utils.h b/server/utils.h
> > new file mode 100644
> > index 0000000..ca8b7f1
> > --- /dev/null
> > +++ b/server/utils.h
> > @@ -0,0 +1,32 @@
> > +/* -*- Mode: C; c-basic-offset: 4; indent-tabs-mode: nil -*- */
> > +/*
> > +   Copyright (C) 2009-2015 Red Hat, Inc.
> > +
> > +   This library is free software; you can redistribute it and/or
> > +   modify it under the terms of the GNU Lesser General Public
> > +   License as published by the Free Software Foundation; either
> > +   version 2.1 of the License, or (at your option) any later
> > version.
> > +
> > +   This library is distributed in the hope that it will be useful,
> > +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> > +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> > GNU
> > +   Lesser General Public License for more details.
> > +
> > +   You should have received a copy of the GNU Lesser General
> > Public
> > +   License along with this library; if not, see
> > <http://www.gnu.org/licenses/>.
> > +*/
> > +#ifndef UTILS_H_
> > +# define UTILS_H_
> > +
> > +#include <time.h>
> > +
> +/* FIXME: consider g_get_monotonic_time (), but in microseconds */
> I would remove this FIXME. Beside that patch looks fine.


Yeah, I don't think that adds anything useful.

> 
> > +static inline red_time_t red_get_monotonic_time(void)
> > +{
> > +    struct timespec time;
> > +
> > +    clock_gettime(CLOCK_MONOTONIC, &time);
> > +    return (red_time_t) time.tv_sec * (1000 * 1000 * 1000) +
> > time.tv_nsec;
> > +}
> > +
> > +#endif /* UTILS_H_ */
> 
> I would like to have another comment on this patch.
> 
> Frediano
> 


Looks fine to me. ACK.