Bug #8093

[patch] gc: avoid unnecessary heap growth

Added by Aman Gupta about 1 year ago. Updated about 1 year ago.

[ruby-core:53393]
Status:Closed
Priority:Normal
Assignee:Narihiro Nakamura
Category:core
Target version:-
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 @@ gcpreparefreeobjects(rbobjspacet *objspace)
gc
marks(objspace);

 before_gc_sweep(objspace);
  • if (objspace->heap.freemin > (heapsused * HEAPOBJLIMIT - objspace->heap.live_num)) {
  • if (objspace->heap.freemin > (heapsused * HEAPOBJLIMIT - objspacelivenum(objspace))) { setheapsincrement(objspace); }

Here heap.livenum and objspacelive_num() are not equivalent.

heap.livenum 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, objspacelivenum() is always greater than heap.livenum. This causes setheaps_increment to be called more often.

I propose adding the heap.livenum counter back, but with a better name: heap.markednum.

diff --git a/gc.c b/gc.c
index bd95073..26bfcf8 100644
--- a/gc.c
+++ b/gc.c
@@ -228,6 +228,7 @@ typedef struct rbobjspace {
struct heaps
freebitmap *freebitmap;
RVALUE *range[2];
struct heapsheader *freed;
+ size
t markednum;
size
t freenum;
size
t freemin;
size
t finalnum;
@@ -2000,7 +2001,7 @@ after
gcsweep(rbobjspacet *objspace)
inc = ATOMIC
SIZEEXCHANGE(mallocincrease, 0);
if (inc > malloclimit) {
malloc
limit +=
- (sizet)((inc - malloclimit) * (double)objspacelivenum(objspace) / (heapsused * HEAPOBJLIMIT));
+ (size
t)((inc - malloclimit) * (double)objspace->heap.markednum / (heapsused * HEAPOBJLIMIT));
if (malloc
limit < initialmalloclimit) malloclimit = initialmalloc_limit;
}

@@ -2073,7 +2074,7 @@ gcpreparefreeobjects(rbobjspacet *objspace)
gc
marks(objspace);

 before_gc_sweep(objspace);
  • if (objspace->heap.freemin > (heapsused * HEAPOBJLIMIT - objspacelivenum(objspace))) {
  • if (objspace->heap.freemin > (heapsused * HEAPOBJLIMIT - objspace->heap.markednum)) { setheaps_increment(objspace); }

@@ -2561,6 +2562,7 @@ gcmarkptr(rbobjspacet *objspace, VALUE ptr)
register uintptrt *bits = GETHEAPBITMAP(ptr);
if (MARKED
INBITMAP(bits, ptr)) return 0;
MARK
INBITMAP(bits, ptr);
+ objspace->heap.marked
num++;
return 1;
}

@@ -2921,6 +2923,7 @@ gcmarks(rbobjspacet *objspace)
objspace->mark
func_data = 0;

 gc_prof_mark_timer_start(objspace);
  • objspace->heap.marked_num = 0;
    objspace->count++;

    SETSTACKEND;


Related issues

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

Associated revisions

Revision 39811
Added by nari about 1 year ago

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

History

#1 Updated by Narihiro Nakamura about 1 year ago

  • Category set to core
  • Assignee set to Narihiro Nakamura

#2 Updated by Narihiro Nakamura about 1 year 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 1 year 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