Determinism patches for stHash and st_random

C/impl/hashTableC.c

@@ -62,6 +62,8 @@ create_hashtable(uint64_t minsize, uint64_t(*hashf)(const void*), int(*eqf)(
     //my additions
     h->keyFree = keyFree;
     h->valueFree = valueFree;
+    h->insertFirst = NULL;
+    h->insertLast = NULL;
     return h;
 }
 
@@ -164,6 +166,7 @@ int64_t hashtable_insert(struct hashtable *h, void *k, void *v) {
     e->v = v;
     e->next = h->table[index];
     h->table[index] = e;
+    hashtable_insertListAppend(h, e);
     return -1;
 }
 
@@ -203,6 +206,7 @@ hashtable_remove(struct hashtable *h, void *k, int64_t freeKey) {
         /* Check hash value to short circuit heavier comparison */
         if ((hashvalue == e->h) && (h->eqfn(k, e->k))) {
             *pE = e->next;
+            hashtable_insertListRemove(h, e);
             h->entrycount--;
             v = e->v;
             if (freeKey) {

C/impl/hashTableC_itr.c

@@ -16,29 +16,18 @@ indexFor(uint64_t tablelength, uint64_t hashvalue) {
 /*****************************************************************************/
 /* hashtable_iterator    - iterator constructor */
 
+/* Walks the insertion-order list, not the buckets, so that the order does not
+ * depend on the numeric value of the keys.  See hashTablePrivateC.h. */
 struct hashtable_itr *
 hashtable_iterator(struct hashtable *h)
 {
-    unsigned int i, tablelength;
     struct hashtable_itr *itr = (struct hashtable_itr *)
         st_malloc(sizeof(struct hashtable_itr));
     if (NULL == itr) return NULL;
     itr->h = h;
-    itr->e = NULL;
+    itr->e = h->insertFirst;
     itr->parent = NULL;
-    tablelength = h->tablelength;
-    itr->index = tablelength;
-    if (0 == h->entrycount) return itr;
-
-    for (i = 0; i < tablelength; i++)
-    {
-        if (NULL != h->table[i])
-        {
-            itr->e = h->table[i];
-            itr->index = i;
-            break;
-        }
-    }
+    itr->index = 0;
     return itr;
 }
 
@@ -49,38 +38,9 @@ hashtable_iterator(struct hashtable *h)
 int
 hashtable_iterator_advance(struct hashtable_itr *itr)
 {
-    unsigned int j,tablelength;
-    struct entry **table;
-    struct entry *next;
     if (NULL == itr->e) return 0; /* stupidity check */
-
-    next = itr->e->next;
-    if (NULL != next)
-    {
-        itr->parent = itr->e;
-        itr->e = next;
-        return -1;
-    }
-    tablelength = itr->h->tablelength;
-    itr->parent = NULL;
-    if (tablelength <= (j = ++(itr->index)))
-    {
-        itr->e = NULL;
-        return 0;
-    }
-    table = itr->h->table;
-    while (NULL == (next = table[j]))
-    {
-        if (++j >= tablelength)
-        {
-            itr->index = tablelength;
-            itr->e = NULL;
-            return 0;
-        }
-    }
-    itr->index = j;
-    itr->e = next;
-    return -1;
+    itr->e = itr->e->insertNext;
+    return (NULL == itr->e) ? 0 : -1;
 }
 
 /*****************************************************************************/
@@ -94,27 +54,25 @@ hashtable_iterator_advance(struct hashtable_itr *itr)
 int
 hashtable_iterator_remove(struct hashtable_itr *itr)
 {
-    struct entry *remember_e, *remember_parent;
+    struct entry *remember_e, **pE;
     int ret;
 
-    /* Do the removal */
-    if (NULL == (itr->parent))
-    {
-        /* element is head of a chain */
-        itr->h->table[itr->index] = itr->e->next;
-    } else {
-        /* element is mid-chain */
-        itr->parent->next = itr->e->next;
-    }
-    /* itr->e is now outside the hashtable */
     remember_e = itr->e;
-    itr->h->entrycount--;
-    freekey(remember_e->k);
 
-    /* Advance the iterator, correcting the parent */
-    remember_parent = itr->parent;
+    /* Unlink from the bucket chain.  The iterator walks the insertion list, so
+     * the chain predecessor has to be looked up rather than remembered. */
+    pE = &(itr->h->table[indexFor(itr->h->tablelength, remember_e->h)]);
+    while (*pE != remember_e) {
+        pE = &((*pE)->next);
+    }
+    *pE = remember_e->next;
+
+    /* Advance before unlinking, so the successor is still reachable */
     ret = hashtable_iterator_advance(itr);
-    if (itr->parent == remember_e) { itr->parent = remember_parent; }
+
+    hashtable_insertListRemove(itr->h, remember_e);
+    itr->h->entrycount--;
+    freekey(remember_e->k);
     free(remember_e);
     return ret;
 }

C/impl/sonLibRandom.c

@@ -8,8 +8,36 @@
 
 const char *RANDOM_EXCEPTION_ID = "RANDOM_EXCEPTION";
 
+/*
+ * The generator state is per-thread rather than per-process.  rand() draws from
+ * one shared sequence, so threads racing for it get a different interleaving on
+ * every run, and anything built from those draws differs run to run.  With the
+ * state thread-local, a parallel loop gets reproducible draws by seeding each
+ * iteration itself, from the work item rather than from the thread -- see the
+ * st_randomSeed calls in cactus's parallel loops.
+ *
+ * Every thread starts from the same default seed, so a single-threaded program
+ * that never calls st_randomSeed sees the same sequence it always did (a fixed
+ * one: rand() was never seeded from the clock either).
+ */
+#if defined(__GNUC__) || defined(__clang__)
+#define ST_THREAD_LOCAL __thread
+#else
+#define ST_THREAD_LOCAL
+#endif
+
+static ST_THREAD_LOCAL uint64_t st_randomState = 1;
+
+/* splitmix64 - small, fast, and passes the usual statistical batteries */
+static uint64_t st_randomNext(void) {
+    uint64_t z = (st_randomState += UINT64_C(0x9E3779B97F4A7C15));
+    z = (z ^ (z >> 30)) * UINT64_C(0xBF58476D1CE4E5B9);
+    z = (z ^ (z >> 27)) * UINT64_C(0x94D049BB133111EB);
+    return z ^ (z >> 31);
+}
+
 void st_randomSeed(int64_t seed) {
-    srand(seed);
+    st_randomState = (uint64_t) seed;
 }
 
 int64_t st_randomInt64(int64_t min, int64_t max) {
@@ -37,8 +65,9 @@ int64_t st_randomInt(int64_t min, int64_t max) {
 }
 
 double st_random(void) {
-    static const double i = RAND_MAX+1.0;
-    double d = rand()/i;
+    // 31 bits, the resolution this returned when it was rand()/(RAND_MAX+1.0).
+    // Callers have been written against values of that granularity, so keep it.
+    double d = (double) (st_randomNext() >> 33) / 2147483648.0; // 2^31
     return d >= 1.0 ? 0.9999 : (d < 0.0 ? 0.0 : d);
 }
 

C/inc/hashTablePrivateC.h

@@ -3,6 +3,7 @@
 #ifndef __HASHTABLE_PRIVATE_CWC22_H__
 #define __HASHTABLE_PRIVATE_CWC22_H__
 
+#include <stddef.h>
 #include "hashTableC.h"
 
 #ifdef __cplusplus
@@ -11,11 +12,22 @@ extern "C" {
 
 /*****************************************************************************/
 
+/*
+ * Entries are on two lists: "next" chains the entries that share a bucket,
+ * while insertNext/insertPrev chain every entry in the table in the order it
+ * was inserted.  Iteration walks the insertion list rather than the buckets,
+ * so that the order a hash yields its entries does not depend on the numeric
+ * value of the keys.  That matters because most hashes here are keyed on
+ * pointers: walking the buckets hands entries back in an order that shifts
+ * with the heap layout, which makes the output of anything downstream of the
+ * iteration differ from run to run.
+ */
 struct entry
 {
     void *k, *v;
     uint64_t h;
     struct entry *next;
+    struct entry *insertNext, *insertPrev;
 };
 
 struct hashtable {
@@ -28,8 +40,35 @@ struct hashtable {
     int (*eqfn) (const void *k1, const void *k2);
     void (*keyFree)(void *);
     void (*valueFree)(void *);
+    struct entry *insertFirst, *insertLast; /* insertion-order list, for iteration */
 };
 
+/*****************************************************************************/
+/* Insertion-order list maintenance, used by hashTableC.c and hashTableC_itr.c */
+static inline void hashtable_insertListAppend(struct hashtable *h, struct entry *e) {
+    e->insertNext = NULL;
+    e->insertPrev = h->insertLast;
+    if (h->insertLast != NULL) {
+        h->insertLast->insertNext = e;
+    } else {
+        h->insertFirst = e;
+    }
+    h->insertLast = e;
+}
+
+static inline void hashtable_insertListRemove(struct hashtable *h, struct entry *e) {
+    if (e->insertPrev != NULL) {
+        e->insertPrev->insertNext = e->insertNext;
+    } else {
+        h->insertFirst = e->insertNext;
+    }
+    if (e->insertNext != NULL) {
+        e->insertNext->insertPrev = e->insertPrev;
+    } else {
+        h->insertLast = e->insertPrev;
+    }
+}
+
 /*****************************************************************************/
 uint64_t
 hashP(struct hashtable *h, void *k);

C/inc/sonLibRandom.h

@@ -27,6 +27,11 @@ extern const char *RANDOM_EXCEPTION_ID;
 
 /*
  * Seed the random number generator.
+ *
+ * The generator state is thread-local, so this seeds the calling thread only.
+ * Code that draws random numbers from a parallel loop and wants a reproducible
+ * result should seed at the top of each iteration with a value derived from the
+ * work item (not from the thread id, which is not stable across runs).
  */
 void st_randomSeed(int64_t seed);