You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@apr.apache.org by Colin <sh...@think42.com> on 2006/11/07 09:05:18 UTC
[Patch] Enhance testatomic.c (new test, higher effectiveness)
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);