Review 'disclaim' add-on partially; fix code in some places again
authorIvan Maidanski <ivmai@mail.ru>
Wed, 28 Sep 2011 09:48:17 +0000 (13:48 +0400)
committerIvan Maidanski <ivmai@mail.ru>
Wed, 28 Sep 2011 09:51:14 +0000 (13:51 +0400)
finalized_mlc.c
include/gc_disclaim.h
include/private/gc_priv.h
include/private/thread_local_alloc.h
mark.c
misc.c
reclaim.c
tests/disclaim_bench.c
thread_local_alloc.c

index da529b4..8627435 100644 (file)
 
 #ifdef ENABLE_DISCLAIM
 
+#include "gc_disclaim.h"
+
 #ifdef THREAD_LOCAL_ALLOC
 # include "private/thread_local_alloc.h"
-#endif
-
-#include "gc_disclaim.h"
+#else
+  STATIC ptr_t * GC_finalized_objfreelist = NULL;
+#endif /* !THREAD_LOCAL_ALLOC */
 
 STATIC int GC_finalized_kind = 0;
 
-GC_INNER ptr_t * GC_finalized_objfreelist = NULL;
-
 STATIC int GC_CALLBACK GC_finalized_disclaim(void *obj, void *cd)
 {
     struct GC_finalizer_closure *fc = *(void **)obj;
@@ -44,20 +44,19 @@ STATIC int GC_CALLBACK GC_finalized_disclaim(void *obj, void *cd)
     return 0;
 }
 
-static int done_init = 0;
+static GC_bool done_init = FALSE;
 
 GC_API void GC_CALL GC_init_finalized_malloc(void)
 {
     DCL_LOCK_STATE;
 
-    GC_init(); // FIXME: Portable client should always do GC_INIT() itself
-
+    GC_init();  /* In case it's not already done.       */
     LOCK();
     if (done_init) {
         UNLOCK();
         return;
     }
-    done_init = 1;
+    done_init = TRUE;
 
     GC_finalized_objfreelist = (ptr_t *)GC_new_free_list_inner();
     GC_finalized_kind = GC_new_kind_inner((void **)GC_finalized_objfreelist,
index f09bc9d..0437f72 100644 (file)
@@ -20,6 +20,9 @@
 /* This API is defined only if the library has been suitably compiled   */
 /* (i.e. with ENABLE_DISCLAIM defined).                                 */
 
+/* Prepare the object kind used for GC_finalized_malloc.                */
+GC_API void GC_CALL GC_init_finalized_malloc(void);
+
 /* Type of a disclaim call-back, always stored along with closure data  */
 /* passed as the second argument.                                       */
 typedef int (GC_CALLBACK * GC_disclaim_proc)(void * /*obj*/, void * /*cd*/);
@@ -43,13 +46,9 @@ struct GC_finalizer_closure {
 
 /* Allocate "size" bytes which is finalized by "fc".  This uses a       */
 /* dedicated object kind with a disclaim procedure, and is more         */
-/* efficient than GC_register_finalizer and friends.  You need to call  */
-/* GC_init_finalized_malloc before using this.                          */
+/* efficient than GC_register_finalizer and friends.                    */
+/* GC_init_finalized_malloc must be called before using this.           */
 GC_API void *GC_CALL GC_finalized_malloc(size_t /*size*/,
                                         struct GC_finalizer_closure * /*fc*/);
 
-/* Prepare the object kind used for GC_finalized_malloc.                */
-GC_API void GC_CALL GC_init_finalized_malloc(void);
-        // FIXME: replace init with enable?
-
 #endif
index 1273b65..f8bd09d 100644 (file)
@@ -815,9 +815,9 @@ struct hblkhdr {
                                 /* Only set with USE_MUNMAP.            */
 #       define FREE_BLK 4       /* Block is free, i.e. not in use.      */
 #       ifdef ENABLE_DISCLAIM
-#           define HAS_DISCLAIM 8
+#         define HAS_DISCLAIM 8
                                 /* This kind has a callback on reclaim. */
-#           define MARK_UNCONDITIONALLY 16
+#         define MARK_UNCONDITIONALLY 0x10
                                 /* Mark from all objects, marked or     */
                                 /* not.  Used to mark objects needed by */
                                 /* reclaim notifier.                    */
@@ -1243,7 +1243,7 @@ GC_EXTERN struct obj_kind {
                         /* is reclaimed, but must also tolerate being   */
                         /* called with object from freelist.  Non-zero  */
                         /* exit prevents object from being reclaimed.   */
-#    define OK_DISCLAIM_INITZ FALSE, NULL, NULL
+#    define OK_DISCLAIM_INITZ /* comma */, FALSE, NULL, NULL
 #  else
 #    define OK_DISCLAIM_INITZ /* empty */
 #  endif /* !ENABLE_DISCLAIM */
@@ -2053,10 +2053,6 @@ GC_EXTERN signed_word GC_bytes_found;
   GC_EXTERN ptr_t * GC_gcjobjfreelist;
 #endif
 
-#ifdef ENABLE_DISCLAIM
-  GC_EXTERN ptr_t * GC_finalized_objfreelist;
-#endif
-
 #if defined(GWW_VDB) && defined(MPROTECT_VDB)
   GC_INNER GC_bool GC_gww_dirty_init(void);
   /* Defined in os_dep.c.  Returns TRUE if GetWriteWatch is available.  */
index 809fff5..3ebb922 100644 (file)
@@ -144,6 +144,10 @@ GC_INNER void GC_destroy_thread_local(GC_tlfs p);
 /* we take care of an individual thread freelist structure.     */
 GC_INNER void GC_mark_thread_local_fls_for(GC_tlfs p);
 
+#ifdef ENABLE_DISCLAIM
+  GC_EXTERN ptr_t * GC_finalized_objfreelist;
+#endif
+
 extern
 #if defined(USE_COMPILER_TLS)
   __thread
diff --git a/mark.c b/mark.c
index 14e6afe..2ea24b9 100644 (file)
--- a/mark.c
+++ b/mark.c
@@ -51,26 +51,26 @@ GC_INNER unsigned GC_n_mark_procs = GC_RESERVED_MARK_PROCS;
 /* It's done here, since we need to deal with mark descriptors.         */
 GC_INNER struct obj_kind GC_obj_kinds[MAXOBJKINDS] = {
 /* PTRFREE */ { &GC_aobjfreelist[0], 0 /* filled in dynamically */,
-                0 | GC_DS_LENGTH, FALSE, FALSE,
-                OK_DISCLAIM_INITZ },
+                0 | GC_DS_LENGTH, FALSE, FALSE
+                /*, */ OK_DISCLAIM_INITZ },
 /* NORMAL  */ { &GC_objfreelist[0], 0,
                 0 | GC_DS_LENGTH,  /* Adjusted in GC_init for EXTRA_BYTES */
-                TRUE /* add length to descr */, TRUE,
-                OK_DISCLAIM_INITZ },
+                TRUE /* add length to descr */, TRUE
+                /*, */ OK_DISCLAIM_INITZ },
 /* UNCOLLECTABLE */
               { &GC_uobjfreelist[0], 0,
-                0 | GC_DS_LENGTH, TRUE /* add length to descr */, TRUE,
-                OK_DISCLAIM_INITZ },
+                0 | GC_DS_LENGTH, TRUE /* add length to descr */, TRUE
+                /*, */ OK_DISCLAIM_INITZ },
 # ifdef ATOMIC_UNCOLLECTABLE
    /* AUNCOLLECTABLE */
               { &GC_auobjfreelist[0], 0,
-                0 | GC_DS_LENGTH, FALSE /* add length to descr */, FALSE,
-                OK_DISCLAIM_INITZ },
+                0 | GC_DS_LENGTH, FALSE /* add length to descr */, FALSE
+                /*, */ OK_DISCLAIM_INITZ },
 # endif
 # ifdef STUBBORN_ALLOC
 /*STUBBORN*/ { (void **)&GC_sobjfreelist[0], 0,
-                0 | GC_DS_LENGTH, TRUE /* add length to descr */, TRUE,
-                OK_DISCLAIM_INITZ },
+                0 | GC_DS_LENGTH, TRUE /* add length to descr */, TRUE
+                /*, */ OK_DISCLAIM_INITZ },
 # endif
 };
 
@@ -1764,7 +1764,7 @@ STATIC void GC_push_marked(struct hblk *h, hdr *hhdr)
     }
 }
 
-#ifdef MARK_UNCONDITIONALLY
+#ifdef ENABLE_DISCLAIM
 /* Unconditionally mark from all objects which have not been reclaimed. */
 /* This is useful in order to retain pointes which are reachable from   */
 /* the disclaim notifiers.                                              */
@@ -1774,8 +1774,8 @@ STATIC void GC_push_marked(struct hblk *h, hdr *hhdr)
 /* first word.  On the other hand, a reclaimed object is a members of   */
 /* free-lists, and thus contains a word-aligned next-pointer as the     */
 /* first word.                                                          */
-void GC_push_unconditionally(struct hblk *h, hdr *hhdr)
-{
+ STATIC void GC_push_unconditionally(struct hblk *h, hdr *hhdr)
+ {
     size_t sz = hhdr -> hb_sz;
     word descr = hhdr -> hb_descr;
     ptr_t p;
@@ -1783,8 +1783,9 @@ void GC_push_unconditionally(struct hblk *h, hdr *hhdr)
     mse * GC_mark_stack_top_reg;
     mse * mark_stack_limit = GC_mark_stack_limit;
 
-    /* Shortcut */
-        if ((0 | GC_DS_LENGTH) == descr) return;
+    if ((0 | GC_DS_LENGTH) == descr)
+        return;
+
     GC_n_rescuing_pages++;
     GC_objects_are_marked = TRUE;
     if (sz > MAXOBJBYTES)
@@ -1797,8 +1798,8 @@ void GC_push_unconditionally(struct hblk *h, hdr *hhdr)
         if ((*(GC_word *)p & 0x3) != 0)
             PUSH_OBJ(p, hhdr, GC_mark_stack_top_reg, mark_stack_limit);
     GC_mark_stack_top = GC_mark_stack_top_reg;
-}
-#endif
+  }
+#endif /* ENABLE_DISCLAIM */
 
 #ifndef GC_DISABLE_INCREMENTAL
   /* Test whether any page in the given block is dirty.   */
@@ -1884,8 +1885,8 @@ STATIC struct hblk * GC_push_next_marked_uncollectable(struct hblk *h)
             GC_push_marked(h, hhdr);
             break;
         }
-#       ifdef MARK_UNCONDITIONALLY
-            if (hhdr -> hb_flags & MARK_UNCONDITIONALLY) {
+#       ifdef ENABLE_DISCLAIM
+            if ((hhdr -> hb_flags & MARK_UNCONDITIONALLY) != 0) {
                 GC_push_unconditionally(h, hhdr);
                 break;
             }
diff --git a/misc.c b/misc.c
index 02b3969..e825705 100644 (file)
--- a/misc.c
+++ b/misc.c
@@ -1478,10 +1478,8 @@ GC_API unsigned GC_CALL GC_new_kind_inner(void **fl, GC_word descr,
     GC_obj_kinds[result].ok_descriptor = descr;
     GC_obj_kinds[result].ok_relocate_descr = adjust;
     GC_obj_kinds[result].ok_init = clear;
-#   ifdef MARK_UNCONDITIONALLY
-        GC_obj_kinds[result].ok_mark_unconditionally = FALSE;
-#   endif
 #   ifdef ENABLE_DISCLAIM
+        GC_obj_kinds[result].ok_mark_unconditionally = FALSE;
         GC_obj_kinds[result].ok_disclaim_proc = 0;
         GC_obj_kinds[result].ok_disclaim_cd = NULL;
 #   endif
index e36aa7f..f8ae8af 100644 (file)
--- a/reclaim.c
+++ b/reclaim.c
@@ -45,8 +45,8 @@ STATIC unsigned GC_n_leaked = 0;
 
 GC_INNER GC_bool GC_have_errors = FALSE;
 
-#if !defined(EAGER_SWEEP) && defined(MARK_UNCONDITIONALLY)
-void GC_reclaim_unconditionally_marked(void);
+#if !defined(EAGER_SWEEP) && defined(ENABLE_DISCLAIM)
+  STATIC void GC_reclaim_unconditionally_marked(void);
 #endif
 
 GC_INLINE void GC_add_leaked(ptr_t leaked)
@@ -302,7 +302,7 @@ GC_INNER ptr_t GC_reclaim_generic(struct hblk * hbp, hdr *hhdr, size_t sz,
       GC_remove_protection(hbp, 1, (hhdr)->hb_descr == 0 /* Pointer-free? */);
 #   endif
 #   ifdef ENABLE_DISCLAIM
-      if (hhdr -> hb_flags & HAS_DISCLAIM) {
+      if ((hhdr -> hb_flags & HAS_DISCLAIM) != 0) {
         result = GC_disclaim_and_reclaim(hbp, hhdr, sz, list, count);
       } else
 #   endif
@@ -381,7 +381,8 @@ STATIC void GC_reclaim_block(struct hblk *hbp, word report_if_found)
             if (report_if_found) {
               GC_add_leaked((ptr_t)hbp);
             } else {
-              size_t blocks = OBJ_SZ_TO_BLOCKS(sz);
+              size_t blocks;
+
 #             ifdef ENABLE_DISCLAIM
                 if (EXPECT(hhdr->hb_flags & HAS_DISCLAIM, 0)) {
                   struct obj_kind *ok = &GC_obj_kinds[hhdr->hb_obj_kind];
@@ -392,6 +393,7 @@ STATIC void GC_reclaim_block(struct hblk *hbp, word report_if_found)
                   }
                 }
 #             endif
+              blocks = OBJ_SZ_TO_BLOCKS(sz);
               if (blocks > 1) {
                 GC_large_allocd_bytes -= blocks * HBLKSIZE;
               }
@@ -646,7 +648,7 @@ GC_INNER void GC_start_reclaim(GC_bool report_if_found)
     /* This is a very stupid thing to do.  We make it possible anyway,  */
     /* so that you can convince yourself that it really is very stupid. */
     GC_reclaim_all((GC_stop_func)0, FALSE);
-# elif defined(MARK_UNCONDITIONALLY)
+# elif defined(ENABLE_DISCLAIM)
     /* However, make sure to clear reclaimable objects of kinds with    */
     /* unconditional marking enabled before we do any significant       */
     /* marking work.                                                    */
@@ -738,13 +740,13 @@ GC_INNER GC_bool GC_reclaim_all(GC_stop_func stop_func, GC_bool ignore_old)
     return(TRUE);
 }
 
-#if !defined(EAGER_SWEEP) && defined(MARK_UNCONDITIONALLY)
+#if !defined(EAGER_SWEEP) && defined(ENABLE_DISCLAIM)
 /* We do an eager sweep on heap blocks where unconditional marking has  */
 /* been enabled, so that any reclaimable objects have been reclaimed    */
 /* before we start marking.  This is a simplified GC_reclaim_all        */
 /* restricted to kinds where ok_mark_unconditionally is true.           */
-void GC_reclaim_unconditionally_marked(void)
-{
+  STATIC void GC_reclaim_unconditionally_marked(void)
+  {
     word sz;
     unsigned kind;
     hdr * hhdr;
@@ -769,5 +771,5 @@ void GC_reclaim_unconditionally_marked(void)
             }
         }
     }
-}
-#endif
+  }
+#endif /* !EAGER_SWEEP && ENABLE_DISCLAIM */
index 2bc3797..fb75d0d 100644 (file)
 #include <stdio.h>
 #include <string.h>
 
-#include <time.h> // FIXME: It would be good not to use timing API by
-                  // default (is it is not quite portable).
-
 #include "atomic_ops.h"
 #include "gc_disclaim.h"
 
 #ifndef AO_HAVE_fetch_and_add1
+
 int main(void)
 {
-    printf("Skipping disclaim_bench since we don't have AO_fetch_and_add1.\n");
+    printf("Skipping disclaim_bench test\n");
     return 0;
 }
+
 #else
 
+#include <time.h>
+
 static AO_t free_count = 0;
 
 typedef struct testobj_s *testobj_t;
@@ -95,19 +96,20 @@ int main(int argc, char **argv)
     keep_arr = GC_MALLOC(sizeof(void *)*KEEP_CNT);
 
     if (argc == 1) {
+// FIXME: Remove this block and invoke this test 3 times from the script
         char *buf = GC_MALLOC(strlen(argv[0]) + 3);
         printf("\t\t\tfin. ratio       time/s    time/fin.\n");
         fflush(stdout);
         for (i = 0; i < 3; ++i) {
             int st;
             sprintf(buf, "%s %d", argv[0], i);
-            st = system(buf); // FIXME: is this available on all targets?
+            st = system(buf);
             if (st != 0)
                 return st;
         }
         return 0;
     }
-    if (argc == 2 && strcmp(argv[1], "--help") == 0) {
+    if (argc < 2 || (argc == 2 && strcmp(argv[1], "--help") == 0)) {
         fprintf(stderr,
                 "Usage: %s FINALIZATION_MODEL\n"
                 "\t0 -- original finalization\n"
@@ -119,8 +121,10 @@ int main(int argc, char **argv)
     if (model < 0 || model > 2)
         exit(2);
     t = -(double)clock();
+    // FIXME: clock() not portable
+    // FIXME: probably it's better to turn on timing only if some macro is on
     for (i = 0; i < ALLOC_CNT; ++i) {
-        int k = rand() % KEEP_CNT;
+        int k = rand() % KEEP_CNT; // FIXME: not protable - see dbg_mlc.c
         keep_arr[k] = testobj_new(model);
     }
 
index 6122c88..2c76ad7 100644 (file)
@@ -32,6 +32,12 @@ GC_key_t GC_thread_key;
 
 static GC_bool keys_initialized;
 
+#ifdef ENABLE_DISCLAIM
+  GC_INNER ptr_t * GC_finalized_objfreelist = NULL;
+        /* This variable is declared here to prevent linking of         */
+        /* finalized_mlc module unless the client uses the latter one.  */
+#endif
+
 /* Return a single nonempty freelist fl to the global one pointed to    */
 /* by gfl.                                                              */