Merge pull request #3423 from facebook/ptime

Refactor timefn, restore support for clock_gettime()

programs/benchzstd.c

@@ -387,12 +387,9 @@ BMK_benchMemAdvancedNoAlloc(
         RDG_genBuffer(compressedBuffer, maxCompressedSize, 0.10, 0.50, 1);
     }
 
-#if defined(UTIL_TIME_USES_C90_CLOCK)
-    if (adv->nbWorkers > 1) {
-        OUTPUTLEVEL(2, "Warning : time measurements restricted to C90 clock_t. \n")
-        OUTPUTLEVEL(2, "Warning : using C90 clock_t leads to incorrect measurements in multithreading mode. \n")
+    if (!UTIL_support_MT_measurements() && adv->nbWorkers > 1) {
+        OUTPUTLEVEL(2, "Warning : time measurements may be incorrect in multithreading mode... \n")
     }
-#endif
 
     /* Bench */
     {   U64 const crcOrig = (adv->mode == BMK_decodeOnly) ? 0 : XXH64(srcBuffer, srcSize, 0);

programs/dibio.c

@@ -274,21 +274,20 @@ static fileStats DiB_fileStats(const char** fileNamesTable, int nbFiles, size_t
     int n;
     memset(&fs, 0, sizeof(fs));
 
-    // We assume that if chunking is requested, the chunk size is < SAMPLESIZE_MAX
+    /* We assume that if chunking is requested, the chunk size is < SAMPLESIZE_MAX */
     assert( chunkSize <= SAMPLESIZE_MAX );
 
     for (n=0; n<nbFiles; n++) {
       S64 const fileSize = DiB_getFileSize(fileNamesTable[n]);
-      // TODO: is there a minimum sample size? What if the file is 1-byte?
+      /* TODO: is there a minimum sample size? What if the file is 1-byte? */
       if (fileSize == 0) {
         DISPLAYLEVEL(3, "Sample file '%s' has zero size, skipping...\n", fileNamesTable[n]);
         continue;
       }
 
       /* the case where we are breaking up files in sample chunks */
-      if (chunkSize > 0)
-      {
-        // TODO: is there a minimum sample size? Can we have a 1-byte sample?
+      if (chunkSize > 0) {
+        /* TODO: is there a minimum sample size? Can we have a 1-byte sample? */
         fs.nbSamples += (int)((fileSize + chunkSize-1) / chunkSize);
         fs.totalSizeToLoad += fileSize;
       }

programs/fileio_common.h

@@ -122,4 +122,4 @@ extern UTIL_time_t g_displayClock;
 #if defined (__cplusplus)
 }
 #endif
-#endif //ZSTD_FILEIO_COMMON_H
+#endif /* ZSTD_FILEIO_COMMON_H */

programs/timefn.c

@@ -12,20 +12,20 @@
 /* ===  Dependencies  === */
 
 #include "timefn.h"
-
+#include "platform.h" /* set _POSIX_C_SOURCE */
+#include <time.h>     /* CLOCK_MONOTONIC, TIME_UTC */
 
 /*-****************************************
 *  Time functions
 ******************************************/
 
 #if defined(_WIN32)   /* Windows */
 
+#include <windows.h>  /* LARGE_INTEGER */
 #include <stdlib.h>   /* abort */
 #include <stdio.h>    /* perror */
 
-UTIL_time_t UTIL_getTime(void) { UTIL_time_t x; QueryPerformanceCounter(&x); return x; }
-
-PTime UTIL_getSpanTimeNano(UTIL_time_t clockStart, UTIL_time_t clockEnd)
+UTIL_time_t UTIL_getTime(void)
 {
     static LARGE_INTEGER ticksPerSecond;
     static int init = 0;
@@ -36,29 +36,60 @@ PTime UTIL_getSpanTimeNano(UTIL_time_t clockStart, UTIL_time_t clockEnd)
         }
         init = 1;
     }
-    return 1000000000ULL*(clockEnd.QuadPart - clockStart.QuadPart)/ticksPerSecond.QuadPart;
+    {   UTIL_time_t r;
+        LARGE_INTEGER x;
+        QueryPerformanceCounter(&x);
+        r.t = (PTime)(x.QuadPart * 1000000000ULL / ticksPerSecond.QuadPart);
+        return r;
+    }
 }
 
 
-
 #elif defined(__APPLE__) && defined(__MACH__)
 
-UTIL_time_t UTIL_getTime(void) { return mach_absolute_time(); }
+#include <mach/mach_time.h> /* mach_timebase_info_data_t, mach_timebase_info, mach_absolute_time */
 
-PTime UTIL_getSpanTimeNano(UTIL_time_t clockStart, UTIL_time_t clockEnd)
+UTIL_time_t UTIL_getTime(void)
 {
     static mach_timebase_info_data_t rate;
     static int init = 0;
     if (!init) {
         mach_timebase_info(&rate);
         init = 1;
     }
-    return ((clockEnd - clockStart) * (PTime)rate.numer) / ((PTime)rate.denom);
+    {   UTIL_time_t r;
+        r.t = mach_absolute_time() * (PTime)rate.numer / (PTime)rate.denom;
+        return r;
+    }
 }
 
+/* POSIX.1-2001 (optional) */
+#elif defined(CLOCK_MONOTONIC)
+
+#include <stdlib.h>   /* abort */
+#include <stdio.h>    /* perror */
 
-/* C11 requires timespec_get, but FreeBSD 11 lacks it, while still claiming C11 compliance.
-   Android also lacks it but does define TIME_UTC. */
+UTIL_time_t UTIL_getTime(void)
+{
+    /* time must be initialized, othersize it may fail msan test.
+     * No good reason, likely a limitation of timespec_get() for some target */
+    struct timespec time = { 0, 0 };
+    if (clock_gettime(CLOCK_MONOTONIC, &time) != 0) {
+        perror("timefn::clock_gettime(CLOCK_MONOTONIC)");
+        abort();
+    }
+    {   UTIL_time_t r;
+        r.t = (PTime)time.tv_sec * 1000000000ULL + (PTime)time.tv_nsec;
+        return r;
+    }
+}
+
+
+/* C11 requires support of timespec_get().
+ * However, FreeBSD 11 claims C11 compliance while lacking timespec_get().
+ * Double confirm timespec_get() support by checking the definition of TIME_UTC.
+ * However, some versions of Android manage to simultanously define TIME_UTC
+ * and lack timespec_get() support... */
 #elif (defined (__STDC_VERSION__) && (__STDC_VERSION__ >= 201112L) /* C11 */) \
     && defined(TIME_UTC) && !defined(__ANDROID__)
 
@@ -69,46 +100,38 @@ UTIL_time_t UTIL_getTime(void)
 {
     /* time must be initialized, othersize it may fail msan test.
      * No good reason, likely a limitation of timespec_get() for some target */
-    UTIL_time_t time = UTIL_TIME_INITIALIZER;
+    struct timespec time = { 0, 0 };
     if (timespec_get(&time, TIME_UTC) != TIME_UTC) {
-        perror("timefn::timespec_get");
+        perror("timefn::timespec_get(TIME_UTC)");
         abort();
     }
-    return time;
-}
-
-static UTIL_time_t UTIL_getSpanTime(UTIL_time_t begin, UTIL_time_t end)
-{
-    UTIL_time_t diff;
-    if (end.tv_nsec < begin.tv_nsec) {
-        diff.tv_sec = (end.tv_sec - 1) - begin.tv_sec;
-        diff.tv_nsec = (end.tv_nsec + 1000000000ULL) - begin.tv_nsec;
-    } else {
-        diff.tv_sec = end.tv_sec - begin.tv_sec;
-        diff.tv_nsec = end.tv_nsec - begin.tv_nsec;
+    {   UTIL_time_t r;
+        r.t = (PTime)time.tv_sec * 1000000000ULL + (PTime)time.tv_nsec;
+        return r;
     }
-    return diff;
-}
-
-PTime UTIL_getSpanTimeNano(UTIL_time_t begin, UTIL_time_t end)
-{
-    UTIL_time_t const diff = UTIL_getSpanTime(begin, end);
-    PTime nano = 0;
-    nano += 1000000000ULL * diff.tv_sec;
-    nano += diff.tv_nsec;
-    return nano;
 }
 
 
 #else   /* relies on standard C90 (note : clock_t produces wrong measurements for multi-threaded workloads) */
 
-UTIL_time_t UTIL_getTime(void) { return clock(); }
-PTime UTIL_getSpanTimeNano(UTIL_time_t clockStart, UTIL_time_t clockEnd) { return 1000000000ULL * (clockEnd - clockStart) / CLOCKS_PER_SEC; }
+UTIL_time_t UTIL_getTime(void)
+{
+    UTIL_time_t r;
+    r.t = (PTime)clock() * 1000000000ULL / CLOCKS_PER_SEC;
+    return r;
+}
+
+#define TIME_MT_MEASUREMENTS_NOT_SUPPORTED
 
 #endif
 
 /* ==== Common functions, valid for all time API ==== */
 
+PTime UTIL_getSpanTimeNano(UTIL_time_t clockStart, UTIL_time_t clockEnd)
+{
+    return clockEnd.t - clockStart.t;
+}
+
 PTime UTIL_getSpanTimeMicro(UTIL_time_t begin, UTIL_time_t end)
 {
     return UTIL_getSpanTimeNano(begin, end) / 1000ULL;
@@ -134,3 +157,12 @@ void UTIL_waitForNextTick(void)
         clockEnd = UTIL_getTime();
     } while (UTIL_getSpanTimeNano(clockStart, clockEnd) == 0);
 }
+
+int UTIL_support_MT_measurements(void)
+{
+# if defined(TIME_MT_MEASUREMENTS_NOT_SUPPORTED)
+    return 0;
+# else
+    return 1;
+# endif
+}

programs/timefn.h

@@ -18,7 +18,7 @@ extern "C" {
 
 
 /*-****************************************
-*  Local Types
+*  Types
 ******************************************/
 
 #if !defined (__VMS) && (defined (__cplusplus) || (defined (__STDC_VERSION__) && (__STDC_VERSION__ >= 199901L) /* C99 */) )
@@ -32,57 +32,35 @@ extern "C" {
   typedef unsigned long long PTime;  /* does not support compilers without long long support */
 #endif
 
-
-
-/*-****************************************
-*  Time types (note: OS dependent)
-******************************************/
-#include <time.h>     /* TIME_UTC, then struct timespec and clock_t */
-
-#if defined(_WIN32)   /* Windows */
-
-    #include <windows.h>   /* LARGE_INTEGER */
-    typedef LARGE_INTEGER UTIL_time_t;
-    #define UTIL_TIME_INITIALIZER { { 0, 0 } }
-
-#elif defined(__APPLE__) && defined(__MACH__)
-
-    #include <mach/mach_time.h>
-    typedef PTime UTIL_time_t;
-    #define UTIL_TIME_INITIALIZER 0
-
-/* C11 requires timespec_get, but FreeBSD 11 lacks it, while still claiming C11 compliance.
-   Android also lacks it but does define TIME_UTC. */
-#elif (defined (__STDC_VERSION__) && (__STDC_VERSION__ >= 201112L) /* C11 */) \
-    && defined(TIME_UTC) && !defined(__ANDROID__)
-
-    typedef struct timespec UTIL_time_t;
-    #define UTIL_TIME_INITIALIZER { 0, 0 }
-
-#else   /* relies on standard C90 (note : clock_t produces wrong measurements for multi-threaded workloads) */
-
-    #define UTIL_TIME_USES_C90_CLOCK
-    typedef clock_t UTIL_time_t;
-    #define UTIL_TIME_INITIALIZER 0
-
-#endif
+/* UTIL_time_t contains a nanosecond time counter.
+ * The absolute value is not meaningful.
+ * It's only valid to compute the difference between 2 measurements. */
+typedef struct { PTime t; } UTIL_time_t;
+#define UTIL_TIME_INITIALIZER { 0 }
 
 
 /*-****************************************
 *  Time functions
 ******************************************/
 
 UTIL_time_t UTIL_getTime(void);
+
+/* Timer resolution can be low on some platforms.
+ * To improve accuracy, it's recommended to wait for a new tick
+ * before starting benchmark measurements */
 void UTIL_waitForNextTick(void);
+/* tells if timefn will return correct time measurements
+ * in presence of multi-threaded workload.
+ * note : this is not the case if only C90 clock_t measurements are available */
+int UTIL_support_MT_measurements(void);
 
 PTime UTIL_getSpanTimeNano(UTIL_time_t clockStart, UTIL_time_t clockEnd);
 PTime UTIL_clockSpanNano(UTIL_time_t clockStart);
 
-#define SEC_TO_MICRO ((PTime)1000000)
 PTime UTIL_getSpanTimeMicro(UTIL_time_t clockStart, UTIL_time_t clockEnd);
 PTime UTIL_clockSpanMicro(UTIL_time_t clockStart);
 
-
+#define SEC_TO_MICRO ((PTime)1000000)  /* nb of microseconds in a second */
 
 
 #if defined (__cplusplus)

tests/decodecorpus.c

@@ -25,21 +25,13 @@
 #include "zdict.h"
 
 /* Direct access to internal compression functions is required */
-#include "zstd_compress.c" /* ZSTD_resetSeqStore, ZSTD_storeSeq, *_TO_OFFBASE, HIST_countFast_wksp, HIST_isError */
+#include "compress/zstd_compress.c" /* ZSTD_resetSeqStore, ZSTD_storeSeq, *_TO_OFFBASE, HIST_countFast_wksp, HIST_isError */
 
 #define XXH_STATIC_LINKING_ONLY
 #include "xxhash.h"     /* XXH64 */
 
-#ifndef MIN
-    #define MIN(a, b) ((a) < (b) ? (a) : (b))
-#endif
-
-#ifndef MAX_PATH
-    #ifdef PATH_MAX
-        #define MAX_PATH PATH_MAX
-    #else
-        #define MAX_PATH 256
-    #endif
+#if !(defined (__cplusplus) || (defined (__STDC_VERSION__) && (__STDC_VERSION__ >= 199901L) /* C99 */))
+# define inline  /* disable */
 #endif
 
 /*-************************************
@@ -71,6 +63,7 @@ static UTIL_time_t g_displayClock = UTIL_TIME_INITIALIZER;
         }                                                                      \
     } while (0)
 
+
 /*-*******************************************************
 *  Random function
 *********************************************************/
@@ -176,6 +169,14 @@ const char* BLOCK_TYPES[] = {"raw", "rle", "compressed"};
 #define MIN_SEQ_LEN (3)
 #define MAX_NB_SEQ ((ZSTD_BLOCKSIZE_MAX + MIN_SEQ_LEN - 1) / MIN_SEQ_LEN)
 
+#ifndef MAX_PATH
+    #ifdef PATH_MAX
+        #define MAX_PATH PATH_MAX
+    #else
+        #define MAX_PATH 256
+    #endif
+#endif
+
 BYTE CONTENT_BUFFER[MAX_DECOMPRESSED_SIZE];
 BYTE FRAME_BUFFER[MAX_DECOMPRESSED_SIZE * 2];
 BYTE LITERAL_BUFFER[ZSTD_BLOCKSIZE_MAX];
@@ -241,6 +242,10 @@ typedef enum {
   gt_block,      /* generate compressed blocks without block/frame headers */
 } genType_e;
 
+#ifndef MIN
+    #define MIN(a, b) ((a) < (b) ? (a) : (b))
+#endif
+
 /*-*******************************************************
 *  Global variables (set from command line)
 *********************************************************/

tests/external_matchfinder.h

@@ -32,4 +32,4 @@ size_t zstreamExternalMatchFinder(
   size_t windowSize
 );
 
-#endif // EXTERNAL_MATCHFINDER
+#endif /* EXTERNAL_MATCHFINDER */