Bug #10537
closedRepeated creation and garbage collection of WeakRef instances against a single object leaks memory
Description
require 'weakref'
a = Object.new
1_000_000.times do
WeakRef.new a
end
GC.start
The above results in Ruby consuming ~150 MB of RAM, all of which can only be freed by dropping a
. This should not be the case - an object being weakly referenced should not itself hold a reference to the WeakRef (or any associated data) pointing at it.
Updated by javawizard (Alex Boyd) over 9 years ago
Data point: tacking a printf
onto the end of define_final0
in gc.c
reveals that a new finalizer is being added to a
for each WeakRef. I expect those are what's hogging memory.
Updated by normalperson (Eric Wong) over 9 years ago
Thanks, the following patch should fix it. Your test runs much in less
than 20s and uses 10M on my older x86-64 machine. It took over 2
minutes before.
(lightly tested, and I'm unfamiliar with the weakmap code):
--- a/gc.c
+++ b/gc.c
@@ -2360,6 +2360,21 @@ define_final0(VALUE obj, VALUE block)
if (st_lookup(finalizer_table, obj, &data)) {
table = (VALUE)data;
+
+ /* avoid duplicate block, table is usually small */
+ {
+ const VALUE *ptr = RARRAY_CONST_PTR(table);
+ long len = RARRAY_LEN(table);
+ long i;
+
+ for (i = 0; i < len; i++, ptr++) {
+ if (rb_funcall(*ptr, idEq, 1, block)) {
+ rb_gc_force_recycle(block);
+ return *ptr;
+ }
+ }
+ }
+
rb_ary_push(table, block);
}
else {
Updated by javawizard (Alex Boyd) over 9 years ago
Getting this:
irb(main):008:0> a = Object.new
=> #<Object:0x007fe1b983a278>
irb(main):009:0> WeakRef.new a
=> #<Object:0x007fe1b983a278>
irb(main):010:0> WeakRef.new a
NotImplementedError: method `==' called on broken T_???(0x10) object (0x007fe1b9842258 flags=0x7fe1b9842270)
from /Users/aboyd/projects/ruby-trunk-install/lib/ruby/2.2.0/weakref.rb:87:in `[]='
from /Users/aboyd/projects/ruby-trunk-install/lib/ruby/2.2.0/weakref.rb:87:in `initialize'
from (irb):10:in `new'
from (irb):10
from bin/irb:11:in `<main>'
No time to dig in further right now, but I'll update when I do.
Updated by normalperson (Eric Wong) over 9 years ago
Thanks for trying. I can not reproduce your broken object issue
with my patch on Linux (x86-64 and i686). "make check" passes, but
maybe something else is missing...
Updated by nobu (Nobuyoshi Nakada) over 9 years ago
- Description updated (diff)
I can't reproduce the error, too.
What about this patch?
diff --git a/gc.c b/gc.c
index 9c0dbef..f4c4e93 100644
--- a/gc.c
+++ b/gc.c
@@ -7869,10 +7869,10 @@ wmap_aset(VALUE self, VALUE wmap, VALUE orig)
TypedData_Get_Struct(self, struct weakmap, &weakmap_type, w);
should_be_finalizable(orig);
should_be_finalizable(wmap);
- define_final0(orig, w->final);
- define_final0(wmap, w->final);
- st_update(w->obj2wmap, (st_data_t)orig, wmap_aset_update, wmap);
- st_insert(w->wmap2obj, (st_data_t)wmap, (st_data_t)orig);
+ if (!st_update(w->obj2wmap, (st_data_t)orig, wmap_aset_update, wmap))
+ define_final0(orig, w->final);
+ if (!st_insert(w->wmap2obj, (st_data_t)wmap, (st_data_t)orig))
+ define_final0(wmap, w->final);
return nonspecial_obj_id(orig);
}
Updated by normalperson (Eric Wong) over 9 years ago
nobu, your patch looks fine to me. However, my original uses less memory
and time on Alex's test on my Phenom II
[ruby-core:66430]
17.64user 0.01system 0:17.64elapsed 100%CPU (0avgtext+0avgdata 9544maxresident)k
[ruby-core:66457]
28.84user 0.01system 0:28.84elapsed 100%CPU (0avgtext+0avgdata 14172maxresident)k
Unfortunately, no word from Alex, yet...
On a related issue, Ctrl-C seems to be ignored with Alex's original
code. I'll have to dig through the threads/signal handling code again
to see what's wrong... (on Linux x86-64)
Updated by javawizard (Alex Boyd) over 9 years ago
I'll see if I can get around to it tonight.
Updated by javawizard (Alex Boyd) over 9 years ago
I'm no longer able to reproduce the issue on trunk, so this looks fine. I tested both patches and Eric's does indeed run faster and with less memory, with 7.3 MB and
real 0m5.028s
user 0m5.001s
sys 0m0.024s
as opposed to ~20 MB and
real 0m18.736s
user 0m18.687s
sys 0m0.041s
but both appear to work equally well other than that.
Updated by normalperson (Eric Wong) over 9 years ago
nobu: any comment? Should I commit my original patch? Thanks.
We'll also need to investigate why Alex's test code does not appear
to handle signal delivery (Ctrl-C) in timely fashion...
Updated by Anonymous over 9 years ago
- Status changed from Open to Closed
- % Done changed from 0 to 100
Applied in changeset r48820.
gc.c (define_final0): avoid duplicate blocks
This prevents excessive memory growth when a WeakRef
is repeatedly created
- gc.c (define_final0): avoid duplicate blocks
[Bug #10537] - test/test_weakref.rb (test_repeated_object_leak): new test
Updated by normalperson (Eric Wong) over 9 years ago
nobu, your patch looks fine to me. However, my original uses less memory
and time on Alex's test on my Phenom II
[ruby-core:66430]
17.64user 0.01system 0:17.64elapsed 100%CPU (0avgtext+0avgdata 9544maxresident)k
[ruby-core:66457]
28.84user 0.01system 0:28.84elapsed 100%CPU (0avgtext+0avgdata 14172maxresident)k
Unfortunately, no word from Alex, yet...
On a related issue, Ctrl-C seems to be ignored with Alex's original
code. I'll have to dig through the threads/signal handling code again
to see what's wrong... (on Linux x86-64)
Updated by nobu (Nobuyoshi Nakada) over 9 years ago
Yes, I confirmed the performance difference too.
I have no idea why it is slow, though.
Updated by usa (Usaku NAKAMURA) over 9 years ago
- Backport changed from 2.0.0: UNKNOWN, 2.1: UNKNOWN to 2.0.0: REQUIRED, 2.1: REQUIRED
Updated by usa (Usaku NAKAMURA) over 9 years ago
- Related to Bug #10618: TestWeakRef#test_repeated_object_leak fails on ARM added