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

[Frediano Ziglio]

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

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