apr-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Colin <share-apa...@think42.com>
Subject [Patch] Enhance testatomic.c (new test, higher effectiveness)
Date Tue, 07 Nov 2006 08:05:18 GMT
Hi,

before I continue with patches for apr_atomic.c (restarting with 
something that I can actually test) a patch for testatomic.c that
makes the following changes:

* Clarify the comment about the non-locked test.
* Add a new test that specifically targets atomic_inc32(); this
  test should currently fail with the Solaris 10 implementation
  on SMP machines; my Solaris 10 patch fixes this, but it has
  not been included yet.
* Make the probability of the concurrent test catching problems
  (this kind of test is always non-deterministic) greater by
  - Increasing the number of iterations per thread by factor 10.
  - Running only 12 threads in parallel instead of 40 (which 
    reduces the scheduling overhead on machines with less than
    40 CPUs, thereby giving the actual tests more concurrency).
  - Only running one test at a time (thereby again increasing
    the effective concurrency per testcase, which increases
    the probability of finding problems).

Regards, Colin


Index: testatomic.c
===================================================================
--- testatomic.c	(revision 472017)
+++ testatomic.c	(working copy)
@@ -14,6 +14,8 @@
  * limitations under the License.
  */
 
+#include <stdlib.h>
+
 #include "testutil.h"
 #include "apr_strings.h"
 #include "apr_thread_proc.h"
@@ -179,6 +181,7 @@
 
 void * APR_THREAD_FUNC thread_func_mutex(apr_thread_t *thd, void *data);
 void * APR_THREAD_FUNC thread_func_atomic(apr_thread_t *thd, void *data);
+void * APR_THREAD_FUNC thread_func_atomic_inc(apr_thread_t *thd, void *data);
 void * APR_THREAD_FUNC thread_func_none(apr_thread_t *thd, void *data);
 
 apr_thread_mutex_t *thread_lock;
@@ -187,11 +190,12 @@
 volatile apr_uint32_t z = 0; /* no locks */
 apr_status_t exit_ret_val = 123; /* just some made up number to check on later */
 
-#define NUM_THREADS 40
-#define NUM_ITERATIONS 20000
+#define NUM_THREADS 12
+#define NUM_ITERATIONS 200000
+
 void * APR_THREAD_FUNC thread_func_mutex(apr_thread_t *thd, void *data)
 {
-    int i;
+    unsigned i;
 
     for (i = 0; i < NUM_ITERATIONS; i++) {
         apr_thread_mutex_lock(thread_lock);
@@ -204,7 +208,7 @@
 
 void * APR_THREAD_FUNC thread_func_atomic(apr_thread_t *thd, void *data)
 {
-    int i;
+    unsigned i;
 
     for (i = 0; i < NUM_ITERATIONS ; i++) {
         apr_atomic_inc32(&y);
@@ -216,9 +220,37 @@
     return NULL;
 }
 
+int test_array_compare(const void * l, const void * r)
+{
+   apr_uint32_t a = *(const apr_uint32_t *)l;
+   apr_uint32_t b = *(const apr_uint32_t *)r;
+
+   if (a < b) {
+      return -1;
+   }
+   else {
+      return a > b;
+   }
+}
+
+apr_uint32_t atomic_test_array[ NUM_ITERATIONS * NUM_THREADS ];
+
+void * APR_THREAD_FUNC thread_func_atomic_inc(apr_thread_t *thd, void *data)
+{
+   unsigned i;
+
+   apr_uint32_t * array = (apr_uint32_t *)data;
+
+   for ( i = 0; i < NUM_ITERATIONS; ++i ) {
+      array[ i ] = apr_atomic_inc32(&y);
+   }
+   apr_thread_exit(thd, exit_ret_val);
+   return NULL;
+}
+
 void * APR_THREAD_FUNC thread_func_none(apr_thread_t *thd, void *data)
 {
-    int i;
+    unsigned i;
 
     for (i = 0; i < NUM_ITERATIONS ; i++) {
         z++;
@@ -230,13 +262,9 @@
 static void test_atomics_threaded(abts_case *tc, void *data)
 {
     apr_thread_t *t1[NUM_THREADS];
-    apr_thread_t *t2[NUM_THREADS];
-    apr_thread_t *t3[NUM_THREADS];
     apr_status_t s1[NUM_THREADS]; 
-    apr_status_t s2[NUM_THREADS];
-    apr_status_t s3[NUM_THREADS];
     apr_status_t rv;
-    int i;
+    unsigned i;
 
 #ifdef HAVE_PTHREAD_SETCONCURRENCY
     pthread_setconcurrency(8);
@@ -246,30 +274,88 @@
     APR_ASSERT_SUCCESS(tc, "Could not create lock", rv);
 
     for (i = 0; i < NUM_THREADS; i++) {
-        apr_status_t r1, r2, r3;
+        apr_status_t r1;
         r1 = apr_thread_create(&t1[i], NULL, thread_func_mutex, NULL, p);
-        r2 = apr_thread_create(&t2[i], NULL, thread_func_atomic, NULL, p);
-        r3 = apr_thread_create(&t3[i], NULL, thread_func_none, NULL, p);
-        ABTS_ASSERT(tc, "Failed creating threads",
-                 r1 == APR_SUCCESS && r2 == APR_SUCCESS && 
-                 r3 == APR_SUCCESS);
+        ABTS_ASSERT(tc, "Failed creating threads", r1 == APR_SUCCESS );
     }
 
     for (i = 0; i < NUM_THREADS; i++) {
         apr_thread_join(&s1[i], t1[i]);
-        apr_thread_join(&s2[i], t2[i]);
-        apr_thread_join(&s3[i], t3[i]);
                      
         ABTS_ASSERT(tc, "Invalid return value from thread_join",
-                 s1[i] == exit_ret_val && s2[i] == exit_ret_val && 
-                 s3[i] == exit_ret_val);
+                    s1[i] == exit_ret_val );
     }
 
     ABTS_INT_EQUAL(tc, x, NUM_THREADS * NUM_ITERATIONS);
+
+    for (i = 0; i < NUM_THREADS; i++) {
+        apr_status_t r1;
+        r1 = apr_thread_create(&t1[i], NULL, thread_func_atomic, NULL, p);
+        ABTS_ASSERT(tc, "Failed creating threads", r1 == APR_SUCCESS );
+    }
+
+    for (i = 0; i < NUM_THREADS; i++) {
+        apr_thread_join(&s1[i], t1[i]);
+                     
+        ABTS_ASSERT(tc, "Invalid return value from thread_join",
+                    s1[i] == exit_ret_val );
+    }
+
     ABTS_INT_EQUAL(tc, apr_atomic_read32(&y), NUM_THREADS * NUM_ITERATIONS);
+
+    y = 0;
+
+    for (i = 0; i < NUM_THREADS * NUM_ITERATIONS; ++i ) {
+       atomic_test_array[ i ] = 0;
+    }       
+
+    for (i = 0; i < NUM_THREADS; i++) {
+        apr_status_t r1;
+        r1 = apr_thread_create(&t1[i], NULL, thread_func_atomic_inc, atomic_test_array
+ i * NUM_ITERATIONS, p);
+        ABTS_ASSERT(tc, "Failed creating threads", r1 == APR_SUCCESS );
+    }
+
+    for (i = 0; i < NUM_THREADS; i++) {
+        apr_thread_join(&s1[i], t1[i]);
+                     
+        ABTS_ASSERT(tc, "Invalid return value from thread_join",
+                    s1[i] == exit_ret_val );
+    }
+
+    qsort( atomic_test_array, NUM_THREADS * NUM_ITERATIONS, sizeof( apr_uint32_t ), &
test_array_compare );
+
+    y = 0;
+
+    for (i = 0; i < NUM_THREADS * NUM_ITERATIONS - 1; ++i ) {
+       y += ( atomic_test_array[ i ] == atomic_test_array[ i + 1 ] );
+    }
+
+    ABTS_ASSERT(tc, "Failed atomic increment return values", y == 0 );
+
+    for (i = 0; i < NUM_THREADS; i++) {
+        apr_status_t r1;
+        r1 = apr_thread_create(&t1[i], NULL, thread_func_none, NULL, p);
+        ABTS_ASSERT(tc, "Failed creating threads", r1 == APR_SUCCESS );
+    }
+
+    for (i = 0; i < NUM_THREADS; i++) {
+        apr_thread_join(&s1[i], t1[i]);
+                     
+        ABTS_ASSERT(tc, "Invalid return value from thread_join",
+                    s1[i] == exit_ret_val );
+    }
+
     /* Comment out this test, because I have no clue what this test is
      * actually telling us.  We are checking something that may or may not
      * be true, and it isn't really testing APR at all.
+
+     * Note by Colin Hirsch: This test was probably introduced by somebody
+     * not to test APR itself, but to test the test; if the non-thread-safe
+     * primitives do not produce a different z then the real tests above
+     * are worthless in the sense that, if nothing goes wrong in the absence
+     * of locking, they do not verify correct locking/synchronisation. The
+     * test will however probably never fail on single CPU machines...
+
     ABTS_ASSERT(tc, "We expect this to fail, because we tried to update "
                  "an integer in a non-thread-safe manner.",
              z != NUM_THREADS * NUM_ITERATIONS);

Mime
View raw message