Issue #8093 has been updated by authorNari (Narihiro Nakamura).


Thank you for the bug report.

I think this patch is acceptable.
ko1-san, what do you think?
# Because r37970 was committed by ko1-san.
----------------------------------------
Bug #8093: [patch] gc: avoid unnecessary heap growth
https://bugs.ruby-lang.org/issues/8093#change-37692

Author: tmm1 (Aman Gupta)
Status: Open
Priority: Normal
Assignee: authorNari (Narihiro Nakamura)
Category: core
Target version: 
ruby -v: ruby 2.1.0dev (2013-03-14 trunk 39748) [x86_64-darwin12.2.1]


In r37970, the following change was made:

> @@ -2063,7 +2070,7 @@ gc_prepare_free_objects(rb_objspace_t *objspace)
>      gc_marks(objspace);
>  
>      before_gc_sweep(objspace);
> -    if (objspace->heap.free_min > (heaps_used * HEAP_OBJ_LIMIT - objspace->heap.live_num)) {
> +    if (objspace->heap.free_min > (heaps_used * HEAP_OBJ_LIMIT - objspace_live_num(objspace))) {
>         set_heaps_increment(objspace);
>      }

Here heap.live_num and objspace_live_num() are not equivalent.

heap.live_num is the number of objects marked during mark phase.
objspace_live_num() is the total number of slots in use, including unmarked slots that haven't been swept yet.

Since this check occurs before sweep begins, objspace_live_num() is always greater than heap.live_num. This causes set_heaps_increment to be called more often.

I propose adding the heap.live_num counter back, but with a better name: heap.marked_num.

diff --git a/gc.c b/gc.c
index bd95073..26bfcf8 100644
--- a/gc.c
+++ b/gc.c
@@ -228,6 +228,7 @@ typedef struct rb_objspace {
         struct heaps_free_bitmap *free_bitmap;
 	RVALUE *range[2];
 	struct heaps_header *freed;
+	size_t marked_num;
 	size_t free_num;
 	size_t free_min;
 	size_t final_num;
@@ -2000,7 +2001,7 @@ after_gc_sweep(rb_objspace_t *objspace)
     inc = ATOMIC_SIZE_EXCHANGE(malloc_increase, 0);
     if (inc > malloc_limit) {
 	malloc_limit +=
-	  (size_t)((inc - malloc_limit) * (double)objspace_live_num(objspace) / (heaps_used * HEAP_OBJ_LIMIT));
+	  (size_t)((inc - malloc_limit) * (double)objspace->heap.marked_num / (heaps_used * HEAP_OBJ_LIMIT));
 	if (malloc_limit < initial_malloc_limit) malloc_limit = initial_malloc_limit;
     }
 
@@ -2073,7 +2074,7 @@ gc_prepare_free_objects(rb_objspace_t *objspace)
     gc_marks(objspace);
 
     before_gc_sweep(objspace);
-    if (objspace->heap.free_min > (heaps_used * HEAP_OBJ_LIMIT - objspace_live_num(objspace))) {
+    if (objspace->heap.free_min > (heaps_used * HEAP_OBJ_LIMIT - objspace->heap.marked_num)) {
 	set_heaps_increment(objspace);
     }
 
@@ -2561,6 +2562,7 @@ gc_mark_ptr(rb_objspace_t *objspace, VALUE ptr)
     register uintptr_t *bits = GET_HEAP_BITMAP(ptr);
     if (MARKED_IN_BITMAP(bits, ptr)) return 0;
     MARK_IN_BITMAP(bits, ptr);
+    objspace->heap.marked_num++;
     return 1;
 }
 
@@ -2921,6 +2923,7 @@ gc_marks(rb_objspace_t *objspace)
     objspace->mark_func_data = 0;
 
     gc_prof_mark_timer_start(objspace);
+    objspace->heap.marked_num = 0;
     objspace->count++;
 
     SET_STACK_END;



-- 
http://bugs.ruby-lang.org/