[Gc] Default stop_func

Ivan Maidanski ivmai at mail.ru
Thu Nov 20 03:08:28 PST 2008


Hi!

Sometimes (for real-time apps) it's necessary to specify stop_func even for implicitly triggered garbage collections.

So, the suggested patch adds GC_set_stop_func() to the API (default stop_func setter) and uses default stop_func inside GC_collect_or_expand (and in other places instead of GC_never_stop_func where possible).

In addition, the patch does 2 minor things:
- makes GC_stopped_mark() and GC_finish_collection() declared as static;
- removes outdated comments about disabled signals (across the whole GC).

Bye.

-------------- next part --------------
diff -ru bdwgc/alloc.c updated/bdwgc/alloc.c
--- bdwgc/alloc.c	2008-11-17 13:39:50.000000000 +0300
+++ updated/bdwgc/alloc.c	2008-11-20 12:04:40.000000000 +0300
@@ -140,8 +140,18 @@
 STATIC int GC_n_attempts = 0;	/* Number of attempts at finishing	*/
 				/* collection within GC_time_limit.	*/
 
+STATIC GC_stop_func GC_default_stop_func = GC_never_stop_func;
+
+GC_API GC_stop_func GC_CALL GC_set_stop_func(GC_stop_func stop_func)
+{
+  GC_stop_func old_stop_func = GC_default_stop_func;
+  if (stop_func != (GC_stop_func)0)
+    GC_default_stop_func = stop_func;
+  return old_stop_func;
+}
+
 #if defined(SMALL_CONFIG) || defined(NO_CLOCK)
-#   define GC_timeout_stop_func GC_never_stop_func
+#   define GC_timeout_stop_func GC_default_stop_func
 #else
   STATIC int GC_CALLBACK GC_timeout_stop_func (void)
   {
@@ -149,6 +159,9 @@
     static unsigned count = 0;
     unsigned long time_diff;
     
+    if ((*GC_default_stop_func)())
+      return(1);
+
     if ((count++ & 3) != 0) return(0);
     GET_TIME(current_time);
     time_diff = MS_TIME_DIFF(current_time,GC_start_time);
@@ -275,6 +288,9 @@
 
 STATIC GC_bool GC_is_full_gc = FALSE;
 
+STATIC GC_bool GC_stopped_mark(GC_stop_func stop_func);
+STATIC void GC_finish_collection(void);
+
 /* 
  * Initiate a garbage collection if appropriate.
  * Choose judiciously
@@ -287,7 +303,8 @@
     GC_ASSERT(I_HOLD_LOCK());
     if (GC_should_collect()) {
         if (!GC_incremental) {
-            GC_gcollect_inner();
+	    /* FIXME: If possible, GC_default_stop_func should be used here */
+            GC_try_to_collect_inner(GC_never_stop_func);
             n_partial_gcs = 0;
             return;
         } else {
@@ -318,6 +335,8 @@
 #	ifndef NO_CLOCK
           if (GC_time_limit != GC_TIME_UNLIMITED) { GET_TIME(GC_start_time); }
 #	endif
+	/* FIXME: If possible, GC_default_stop_func should be	*/
+	/* used instead of GC_never_stop_func here.		*/
         if (GC_stopped_mark(GC_time_limit == GC_TIME_UNLIMITED? 
 			    GC_never_stop_func : GC_timeout_stop_func)) {
 #           ifdef SAVE_CALL_CHAIN
@@ -335,8 +354,8 @@
 
 
 /*
- * Stop the world garbage collection.  Assumes lock held, signals disabled.
- * If stop_func is not GC_never_stop_func, then abort if stop_func returns TRUE.
+ * Stop the world garbage collection.  Assumes lock held. If stop_func is
+ * not GC_never_stop_func then abort if stop_func returns TRUE.
  * Return TRUE if we successfully completed the collection.
  */
 GC_bool GC_try_to_collect_inner(GC_stop_func stop_func)
@@ -455,6 +474,8 @@
 		    break;
 		  }
 		} else {
+		  /* FIXME: If possible, GC_default_stop_func should be	*/
+		  /* used here.						*/
 		  (void)GC_stopped_mark(GC_never_stop_func);
 		}
     	        GC_finish_collection();
@@ -503,12 +524,11 @@
 #endif
 
 /*
- * Assumes lock is held, signals are disabled.
- * We stop the world.
+ * Assumes lock is held.  We stop the world and mark from all roots.
  * If stop_func() ever returns TRUE, we may fail and return FALSE.
  * Increment GC_gc_no if we succeed.
  */
-GC_bool GC_stopped_mark(GC_stop_func stop_func)
+STATIC GC_bool GC_stopped_mark(GC_stop_func stop_func)
 {
     unsigned i;
     int dummy;
@@ -691,9 +711,9 @@
 void GC_traverse_back_graph(void);
 #endif
 
-/* Finish up a collection.  Assumes lock is held, signals are disabled,	*/
-/* but the world is otherwise running.					*/
-void GC_finish_collection(void)
+/* Finish up a collection.  Assumes mark bits are consistent, lock is	*/
+/* held, but the world is otherwise running.				*/
+STATIC void GC_finish_collection(void)
 {
 #   ifndef SMALL_CONFIG
       CLOCK_TYPE start_time;
@@ -845,6 +865,7 @@
     DCL_LOCK_STATE;
     
     if (!GC_is_initialized) GC_init();
+    GC_ASSERT(stop_func != 0);
     if (GC_debugging_started) GC_print_all_smashed();
     GC_INVOKE_FINALIZERS();
     LOCK();
@@ -863,7 +884,7 @@
 
 GC_API void GC_CALL GC_gcollect(void)
 {
-    (void)GC_try_to_collect(GC_never_stop_func);
+    (void)GC_try_to_collect(GC_default_stop_func);
     if (GC_have_errors) GC_print_all_errors();
 }
 
@@ -1081,10 +1102,13 @@
 
 GC_bool GC_collect_or_expand(word needed_blocks, GC_bool ignore_off_page)
 {
-    if (!GC_incremental && !GC_dont_gc &&
-	((GC_dont_expand && GC_bytes_allocd > 0) || GC_should_collect())) {
-      GC_gcollect_inner();
-    } else {
+    GC_bool gc_not_stopped = TRUE;
+    if (GC_incremental || GC_dont_gc ||
+	((!GC_dont_expand || GC_bytes_allocd == 0) && !GC_should_collect()) ||
+	(gc_not_stopped = GC_try_to_collect_inner(GC_bytes_allocd > 0 ?
+					GC_default_stop_func :
+					GC_never_stop_func)) == FALSE) {
+
       word blocks_to_get = GC_heapsize/(HBLKSIZE*GC_free_space_divisor)
       			   + needed_blocks;
       
@@ -1108,7 +1132,11 @@
       }
       if (!GC_expand_hp_inner(blocks_to_get)
         && !GC_expand_hp_inner(needed_blocks)) {
-      	if (GC_fail_count++ < GC_max_retries) {
+        if (gc_not_stopped == FALSE) {
+	    /* Don't increment GC_fail_count here (and no warning).	*/
+	    GC_gcollect_inner();
+	    GC_ASSERT(GC_bytes_allocd == 0);
+	} else if (GC_fail_count++ < GC_max_retries) {
       	    WARN("Out of Memory!  Trying to continue ...\n", 0);
 	    GC_gcollect_inner();
 	} else {
@@ -1130,8 +1158,7 @@
  * Make sure the object free list for size gran (in granules) is not empty.
  * Return a pointer to the first object on the free list.
  * The object MUST BE REMOVED FROM THE FREE LIST BY THE CALLER.
- * Assumes we hold the allocator lock and signals are disabled.
- *
+ * Assumes we hold the allocator lock.
  */
 ptr_t GC_allocobj(size_t gran, int kind)
 {
diff -ru bdwgc/dbg_mlc.c updated/bdwgc/dbg_mlc.c
--- bdwgc/dbg_mlc.c	2008-10-31 13:46:24.000000000 +0300
+++ updated/bdwgc/dbg_mlc.c	2008-11-20 12:05:32.000000000 +0300
@@ -247,9 +247,6 @@
     register word * result = (word *)((oh *)p + 1);
     DCL_LOCK_STATE;
     
-    /* There is some argument that we should dissble signals here.	*/
-    /* But that's expensive.  And this way things should only appear	*/
-    /* inconsistent while we're in the handler.				*/
     LOCK();
     GC_ASSERT(GC_size(p) >= sizeof(oh) + sz);
     GC_ASSERT(!(SMALL_OBJ(sz) && CROSSES_HBLK(p, sz)));
@@ -279,9 +276,6 @@
 {
     register word * result = (word *)((oh *)p + 1);
     
-    /* There is some argument that we should disable signals here.	*/
-    /* But that's expensive.  And this way things should only appear	*/
-    /* inconsistent while we're in the handler.				*/
     GC_ASSERT(GC_size(p) >= sizeof(oh) + sz);
     GC_ASSERT(!(SMALL_OBJ(sz) && CROSSES_HBLK(p, sz)));
 #   ifdef KEEP_BACK_PTRS
diff -ru bdwgc/finalize.c updated/bdwgc/finalize.c
--- bdwgc/finalize.c	2008-10-25 15:25:42.000000000 +0400
+++ updated/bdwgc/finalize.c	2008-11-20 12:00:32.000000000 +0300
@@ -99,7 +99,7 @@
 /* size.  May be a no-op.						*/
 /* *table is a pointer to an array of hash headers.  If we succeed, we	*/
 /* update both *table and *log_size_ptr.				*/
-/* Lock is held.  Signals are disabled.					*/
+/* Lock is held.							*/
 STATIC void GC_grow_table(struct hash_chain_entry ***table,
 			  signed_word *log_size_ptr)
 {
@@ -297,9 +297,6 @@
 
 
 /* Register a finalization function.  See gc.h for details.	*/
-/* in the nonthreads case, we try to avoid disabling signals,	*/
-/* since it can be expensive.  Threads packages typically	*/
-/* make it cheaper.						*/
 /* The last parameter is a procedure that determines		*/
 /* marking for finalization ordering.  Any objects marked	*/
 /* by that procedure will be guaranteed to not have been	*/
@@ -328,8 +325,7 @@
 	    	          (1 << (unsigned)log_fo_table_size));
 	}
     }
-    /* in the THREADS case signals are disabled and we hold allocation	*/
-    /* lock; otherwise neither is true.  Proceed carefully.		*/
+    /* in the THREADS case we hold allocation lock.		*/
     base = (ptr_t)obj;
     index = HASH2(base, log_fo_table_size);
     prev_fo = 0; curr_fo = fo_head[index];
@@ -655,8 +651,7 @@
 
 #ifndef JAVA_FINALIZATION_NOT_NEEDED
 
-/* Enqueue all remaining finalizers to be run - Assumes lock is
- * held, and signals are disabled */
+/* Enqueue all remaining finalizers to be run - Assumes lock is held.	*/
 STATIC void GC_enqueue_all_finalizers(void)
 {
     struct finalizable_object * curr_fo, * prev_fo, * next_fo;
diff -ru bdwgc/include/gc.h updated/bdwgc/include/gc.h
--- bdwgc/include/gc.h	2008-11-17 13:03:28.000000000 +0300
+++ updated/bdwgc/include/gc.h	2008-11-20 00:30:56.000000000 +0300
@@ -405,6 +405,12 @@
 typedef int (GC_CALLBACK * GC_stop_func)(void);
 GC_API int GC_CALL GC_try_to_collect(GC_stop_func stop_func);
 
+/* Set default stop_func.  Returns old stop_func.  With 0 argument,	*/
+/* default stop_func remains unchanged.  Default stop_func is used by	*/
+/* GC_gcollect() and by implicitly trigged collections (except for the	*/
+/* case when handling out of memory).					*/
+GC_API GC_stop_func GC_CALL GC_set_stop_func(GC_stop_func stop_func);
+
 /* Return the number of bytes in the heap.  Excludes collector private	*/
 /* data structures.  Includes empty blocks and fragmentation loss.	*/
 /* Includes some pages that were allocated but never written.		*/
diff -ru bdwgc/include/private/gc_priv.h updated/bdwgc/include/private/gc_priv.h
--- bdwgc/include/private/gc_priv.h	2008-11-17 13:22:32.000000000 +0300
+++ updated/bdwgc/include/private/gc_priv.h	2008-11-20 12:02:28.000000000 +0300
@@ -1468,9 +1468,6 @@
   		/* Ditto, but also mark from clean pages.	*/
 struct hblk * GC_push_next_marked_uncollectable(struct hblk * h);
   		/* Ditto, but mark only from uncollectable pages.	*/
-GC_bool GC_stopped_mark(GC_stop_func stop_func);
- 			/* Stop world and mark from all roots	*/
-  			/* and rescuers.			*/
 void GC_clear_hdr_marks(hdr * hhdr);
 				    /* Clear the mark bits in a header */
 void GC_set_hdr_marks(hdr * hhdr);
@@ -1573,8 +1570,7 @@
   				/* Return FALSE on failure.		*/
 void GC_register_displacement_inner(size_t offset);
   				/* Version of GC_register_displacement	*/
-  				/* that assumes lock is already held	*/
-  				/* and signals are already disabled.	*/
+  				/* that assumes lock is already held.	*/
 
 void GC_initialize_offsets(void);
 				/* Initialize GC_valid_offsets,		*/
@@ -1653,15 +1649,11 @@
 GC_bool GC_try_to_collect_inner(GC_stop_func f);
 
 				/* Collect; caller must have acquired	*/
-				/* lock and disabled signals.		*/
-				/* Collection is aborted if f returns	*/
-				/* TRUE.  Returns TRUE if it completes	*/
-				/* successfully.			*/
+				/* lock.  Collection is aborted if f	*/
+				/* returns TRUE.  Returns TRUE if it	*/
+				/* completes successfully.		*/
 # define GC_gcollect_inner() \
 	(void) GC_try_to_collect_inner(GC_never_stop_func)
-void GC_finish_collection(void);
- 				/* Finish collection.  Mark bits are	*/
-  				/* consistent and lock is still held.	*/
 GC_bool GC_collect_or_expand(word needed_blocks, GC_bool ignore_off_page);
   				/* Collect or expand heap in an attempt */
   				/* make the indicated number of free	*/
diff -ru bdwgc/malloc.c updated/bdwgc/malloc.c
--- bdwgc/malloc.c	2008-10-31 22:43:38.000000000 +0300
+++ updated/bdwgc/malloc.c	2008-11-20 11:54:08.000000000 +0300
@@ -266,7 +266,6 @@
             UNLOCK();
             return(GENERAL_MALLOC((word)lb, NORMAL));
         }
-        /* See above comment on signals.	*/
 	GC_ASSERT(0 == obj_link(op)
 		  || ((word)obj_link(op)
 		  	<= (word)GC_greatest_plausible_heap_addr
diff -ru bdwgc/mallocx.c updated/bdwgc/mallocx.c
--- bdwgc/mallocx.c	2008-10-31 22:44:58.000000000 +0300
+++ updated/bdwgc/mallocx.c	2008-11-20 11:54:26.000000000 +0300
@@ -453,7 +453,6 @@
 	opp = &(GC_uobjfreelist[lg]);
 	LOCK();
         if( (op = *opp) != 0 ) {
-            /* See above comment on signals.	*/
             *opp = obj_link(op);
             obj_link(op) = 0;
             GC_bytes_allocd += GRANULES_TO_BYTES(lg);
@@ -541,7 +540,6 @@
 	opp = &(GC_auobjfreelist[lg]);
 	LOCK();
         if( (op = *opp) != 0 ) {
-            /* See above comment on signals.	*/
             *opp = obj_link(op);
             obj_link(op) = 0;
             GC_bytes_allocd += GRANULES_TO_BYTES(lg);
diff -ru bdwgc/os_dep.c updated/bdwgc/os_dep.c
--- bdwgc/os_dep.c	2008-11-11 14:43:12.000000000 +0300
+++ updated/bdwgc/os_dep.c	2008-11-20 11:55:14.000000000 +0300
@@ -2348,8 +2348,7 @@
 
 # ifdef DEFAULT_VDB
 
-/* All of the following assume the allocation lock is held, and	*/
-/* signals are disabled.					*/
+/* All of the following assume the allocation lock is held.	*/
 
 /* The client asserts that unallocated pages in the heap are never	*/
 /* written.								*/


More information about the Gc mailing list