Bug #8093

[patch] gc: avoid unnecessary heap growth

Added by Aman Gupta about 2 years ago. Updated about 2 years ago.

[ruby-core:53393]
Status:Closed
Priority:Normal
Assignee:Narihiro Nakamura
ruby -v:ruby 2.1.0dev (2013-03-14 trunk 39748) [x86_64-darwin12.2.1] Backport:

Description

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;


Related issues

Related to Backport200 - Backport #8146: Backport r39811 Closed 03/22/2013

Associated revisions

Revision 39811
Added by nari about 2 years ago

  • gc.c: Avoid unnecessary heap growth. patched by tmm1(Aman Gupta). [Bug #8093]

Revision 39811
Added by nari about 2 years ago

  • gc.c: Avoid unnecessary heap growth. patched by tmm1(Aman Gupta). [Bug #8093]

History

#1 Updated by Narihiro Nakamura about 2 years ago

  • Category set to core
  • Assignee set to Narihiro Nakamura

#2 Updated by Narihiro Nakamura about 2 years ago

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.

#3 Updated by Narihiro Nakamura about 2 years ago

  • Status changed from Open to Closed
  • % Done changed from 0 to 100

This issue was solved with changeset r39811.
Aman, thank you for reporting this issue.
Your contribution to Ruby is greatly appreciated.
May Ruby be with you.


  • gc.c: Avoid unnecessary heap growth. patched by tmm1(Aman Gupta). [Bug #8093]

Also available in: Atom PDF