[Gc] gctest: use of AO_fetch_and_add1

Boehm, Hans hans.boehm at hp.com
Wed May 27 16:15:21 PDT 2009


Thanks.  That helps.

Committed, except for

@@ -1308,12 +1309,13 @@
     	           (GC_bytes_allocd + GC_bytes_allocd_before_gc));
     (void)GC_printf("Final heap size is %lu bytes\n",
     		    (unsigned long)GC_get_heap_size());
-    if (GC_bytes_allocd + GC_bytes_allocd_before_gc
+    if (GC_bytes_allocd + GC_bytes_allocd_before_gc < n_tests *
 #   ifdef VERY_SMALL_CONFIG
-        < 2700000*n_tests) {
+        2700000
 #   else
-        < 33500000*n_tests) {
+        33500000
 #   endif
+        ) {
         (void)GC_printf("Incorrect execution - missed some allocations\n");
         FAIL;
     }

The amount of allocation should go up with the number of tests run.  I think this code is at least a little flakey, since the various counters should be maintained using atomic updates, or preferably in thread local variables.  Thus these counts are likely to be at least somewhat inaccurate, and we could see spurious test failures.  I haven't noticed that in practice, probably because the counts we use are rough lower bounds anyway.  That problem is worth fixing, though I think it can only result in spurious test failures.  But I'm not sure the patch above improves matters, since it leaves the data races in the code.

Hans 

> -----Original Message-----
> From: gc-bounces at napali.hpl.hp.com 
> [mailto:gc-bounces at napali.hpl.hp.com] On Behalf Of Ivan Maidanski
> Sent: Wednesday, May 27, 2009 1:48 AM
> To: gc at napali.hpl.hp.com
> Subject: Re[2]: [Gc] gctest: use of AO_fetch_and_add1
> 
> Hi!
> 
> "Boehm, Hans" <hans.boehm at hp.com> wrote:
> > Thanks.  Tested on an armv5b machine (where the 
> fetch_and_add version by itself failed) and committed.
> 
> OK.
> > 
> > Hans
> > 
> > > ...
> 
> To simplify your "process", I've prepared a cumulative patch 
> (resembling diff28, diff32, diff33, diff38, diff68 partly) 
> for test.c (and trace_test.c) against the current CVS. It 
> contains no "functional" changes and could be (IMHO) easily committed.
> 
> ChangeLog entries:
> 
>         * test.c (fork_a_thread, reverse_test, alloc8bytes, tree_test,
>         typed_test, run_one_test, check_heap_stats, main, 
> test): Replace
>         all K&R-style function definitions with ANSI C ones.
>         * trace_test.c (main): Ditto.
>         * test.c (GC_COND_INIT): Define as GC_INIT() also in case of
>         THREAD_LOCAL_ALLOC.
>         * test.c (reverse_test): Call fork_a_thread() only if 
> GC_PTHREADS
>         or GC_WIN32_THREADS; remove fork_a_thread() macros definition.
>         * test.c (reverse_test): Use "volatile" when clearing 
> "b" and "c"
>         local variables (to suppress "assigned value is never used"
>         compiler warning).
>         * test.c (tree_test): Use public GC_noop1() instead of private
>         GC_noop().
>         * test.c (typed_test): Ditto.
>         * test.c (check_heap_stats): Define and assign value to
>         "late_finalize_count" local variable only if its value is used
>         (if FINALIZE_ON_DEMAND defined).
>         * test.c (main): Remove DJGPP-specific initialization of
>         GC_stackbottom (not needed anymore, handled in gcconfig.h).
>         * trace_test.c: Guard #define GC_DEBUG with #ifndef.
>         * trace_test.c: Include "gc_backptr.h".
>         * trace_test.c (main): Call GC_INIT().
>         * trace_test.c (main): Add "return 0" statement.
> 	
> Bye.
> 


More information about the Gc mailing list