[Piglit] [PATCH 03/10] piglit-util: add piglit_get_microseconds

[Chad Versace]

On 10/15/2013 04:33 PM, Jordan Justen wrote:
> Signed-off-by: Jordan Justen <jordan.l.justen at intel.com>
> Cc: Chad Versace <chad.versace at linux.intel.com>
> ---
> Chad, this conflicts with your "Detect if system has POSIX clocks" patch,
> but it could be tweaked to follow your version.

I'm not opposed to defining a piglit_get_microseconds(). However, I'd
rather layer it on top of my already submitted POSIX clocks patch.

Issues below...

>
>   CMakeLists.txt           |    6 ++++++
>   tests/util/piglit-util.c |   17 +++++++++++++++++
>   tests/util/piglit-util.h |    8 ++++++++
>   3 files changed, 31 insertions(+)
>
> diff --git a/CMakeLists.txt b/CMakeLists.txt
> index 4ec5ddf..c876a05 100644
> --- a/CMakeLists.txt
> +++ b/CMakeLists.txt
> @@ -6,6 +6,7 @@ INCLUDE (CheckCCompilerFlag)
>   INCLUDE (CheckCXXCompilerFlag)
>   INCLUDE (CheckFunctionExists)
>   INCLUDE (CheckIncludeFile)
> +INCLUDE (CheckLibraryExists)
>   INCLUDE (FindPkgConfig)
>
>   project (piglit)
> @@ -294,6 +295,11 @@ check_function_exists(strchrnul HAVE_STRCHRNUL)
>   check_function_exists(strndup   HAVE_STRNDUP)
>   check_function_exists(fopen_s   HAVE_FOPEN_S)
>   check_function_exists(setrlimit HAVE_SETRLIMIT)
> +check_function_exists(clock_gettime HAVE_CLOCK_GETTIME_FUNC)

I just inspected CMake's implementation of check_function_exists(),
and now I am strongly against using it anywhere. If you saw its implementation,
you would likely agree too. You find it in /usr/share/cmake/Modules/CheckFunctionExists.{c,cmake}.

> +check_library_exists(rt clock_gettime "time.h" HAVE_CLOCK_GETTIME_RT)
> +if(HAVE_CLOCK_GETTIME_FUNC OR HAVE_CLOCK_GETTIME_RT)
> +    add_definitions(-DHAVE_CLOCK_GETTIME)
> +endif()
>
>   check_include_file(sys/time.h  HAVE_SYS_TIME_H)
>   check_include_file(sys/types.h HAVE_SYS_TYPES_H)
> diff --git a/tests/util/piglit-util.c b/tests/util/piglit-util.c
> index 71d55a7..18df0b6 100644
> --- a/tests/util/piglit-util.c
> +++ b/tests/util/piglit-util.c
> @@ -33,6 +33,10 @@
>   #include <string.h>
>   #include <errno.h>
>
> +#ifdef HAVE_CLOCK_GETTIME
> +#include <time.h>
> +#endif
> +
>   #include "config.h"
>   #if defined(HAVE_SYS_TIME_H) && defined(HAVE_SYS_RESOURCE_H) && defined(HAVE_SETRLIMIT)
>   #include <sys/time.h>
> @@ -462,3 +466,16 @@ write_null:
>   	va_end(va);
>   	return size_written;
>   }
> +
> +uint64_t
> +piglit_get_microseconds(void)
> +{
> +#ifdef HAVE_CLOCK_GETTIME
> +	struct timespec t;
> +	clock_gettime(CLOCK_MONOTONIC, &t);
> +	return (t.tv_sec * 1000000) + (t.tv_nsec / 1000);
> +#else
> +	return (uint64_t) 0;
> +#endif
> +}

I'm opposed to implementing broken functions. If this function is not yet
implemented for some system, then tests that require it should either skip
or not even build. This patch's approach, though, causes such tests to fail.

> +
> diff --git a/tests/util/piglit-util.h b/tests/util/piglit-util.h
> index 52f053e..6f64ccf 100644
> --- a/tests/util/piglit-util.h
> +++ b/tests/util/piglit-util.h
> @@ -174,6 +174,14 @@ piglit_source_dir(void);
>   size_t
>   piglit_join_paths(char buf[], size_t buf_size, int n, ...);
>
> +/**
> + * \brief Get a monotonically increasing time in microseconds
> + *
> + * This time can be used for relative time measurements.
> + */
> +uint64_t
> +piglit_get_microseconds(void);
> +
>   #ifdef __cplusplus
>   } /* end extern "C" */
>   #endif
>