Feature #12416: struct rb_id_table lacks mark function

• Status: Open
• Assignee: -
• Target version: -
• Tags: core, maybenotbug

[shyouhei (Shyouhei Urabe)]

From 368b63af3b9d4205048f19058453cddda0cf242c Mon Sep 17 00:00:00 2001
From: "Urabe, Shyouhei" <[e-mail]>
Date: Mon, 23 May 2016 16:03:04 +0900
Subject: [PATCH 1/1] [Bug] rb_id_table must come with mark function.

The struct rb_id_table can hold arbitrary VALUE values.  Now that this
struct is reachable form Ruby's object space, it must understand what GC
requests to it.

	* id_table.c (rb_id_table_mark): new function to avoid GC leak for
	  id tables.  This function applies against all implementations.

	* id_table.h (rb_id_table_mark): ditto.

Signed-off-by: Urabe, Shyouhei <[e-mail]>
---
 id_table.c | 17 +++++++++++++++++
 id_table.h |  1 +
 iseq.c     |  1 +
 3 files changed, 19 insertions(+)

diff --git a/id_table.c b/id_table.c
index b8111aa..0649d0b 100644
--- a/id_table.c
+++ b/id_table.c
@@ -1586,3 +1586,20 @@ show_impl(void)
     fprintf(stderr, "impl: %d\n", ID_TABLE_IMPL);
 }
 #endif
+
+/* impl-agnostic functions */
+
+static enum rb_id_table_iterator_result
+rb_id_table_mark_generic_i(VALUE v, void *ign)
+{
+    rb_gc_mark(v);
+    return ID_TABLE_CONTINUE;
+}
+
+void
+rb_id_table_mark(struct rb_id_table *tbl)
+{
+    if (tbl) {
+	rb_id_table_foreach_values(tbl, rb_id_table_mark_generic_i, NULL);
+    }
+}
diff --git a/id_table.h b/id_table.h
index 4b4eb6f..ffdccd6 100644
--- a/id_table.h
+++ b/id_table.h
@@ -12,6 +12,7 @@ enum rb_id_table_iterator_result {
 };
 
 struct rb_id_table *rb_id_table_create(size_t size);
+void rb_id_table_mark(struct rb_id_table *tbl);
 void rb_id_table_free(struct rb_id_table *tbl);
 void rb_id_table_clear(struct rb_id_table *tbl);
 
diff --git a/iseq.c b/iseq.c
index 28d35eb..6d68ea4 100644
--- a/iseq.c
+++ b/iseq.c
@@ -128,6 +128,7 @@ rb_iseq_mark(const rb_iseq_t *iseq)
 	RUBY_MARK_UNLESS_NULL(compile_data->mark_ary);
 	RUBY_MARK_UNLESS_NULL(compile_data->err_info);
 	RUBY_MARK_UNLESS_NULL(compile_data->catch_table_ary);
+	rb_id_table_mark(compile_data->ivar_cache_table);
     }
 
     RUBY_MARK_LEAVE("iseq");
-- 
2.8.2

[normalperson (Eric Wong)]

[e-mail] wrote:

Bug #12416: struct rb_id_table lacks mark function
https://bugs.ruby-lang.org/issues/12416

The struct rb_id_table can hold arbitrary VALUE values.

Can, but currently does not (AFAIK).

Now that this
struct is reachable form Ruby's object space, it must understand what GC
requests to it.

Do you have plans to hold arbitrary VALUE objects?

+++ b/iseq.c
@@ -128,6 +128,7 @@ rb_iseq_mark(const rb_iseq_t *iseq)
RUBY_MARK_UNLESS_NULL(compile_data->mark_ary);
RUBY_MARK_UNLESS_NULL(compile_data->err_info);
RUBY_MARK_UNLESS_NULL(compile_data->catch_table_ary);

• rb_id_table_mark(compile_data->ivar_cache_table);

Right now, ivar_cache_table only holds Fixnum which does
not need marking. Even with generational GC, I still prefer
we not waste CPU cycles walking through tables for noops.

[shyouhei (Shyouhei Urabe)]

On 05/23/2016 05:07 PM, Eric Wong wrote:

Do you have plans to hold arbitrary VALUE objects?

Yes. I plan to post a feature request with a patch, that has a
hunk to store an array into an rb_id_table.

+++ b/iseq.c
@@ -128,6 +128,7 @@ rb_iseq_mark(const rb_iseq_t *iseq)
RUBY_MARK_UNLESS_NULL(compile_data->mark_ary);
RUBY_MARK_UNLESS_NULL(compile_data->err_info);
RUBY_MARK_UNLESS_NULL(compile_data->catch_table_ary);

• rb_id_table_mark(compile_data->ivar_cache_table);

Right now, ivar_cache_table only holds Fixnum which does
not need marking. Even with generational GC, I still prefer
we not waste CPU cycles walking through tables for noops.

OK then, I give up this iseq.c hunk.

Given the ability for this struct to hold GC-managed objects, I
guess it is only a matter of time if someone actually do. People
can use id tables that way now already with a custom mark
function. I think it is a good idea to provide a default one for
them, even if it has no actual usage today.

[jeremyevans0 (Jeremy Evans)]

• Tracker changed from Bug to Feature
• ruby -v deleted (ruby 2.4.0dev (2016-05-23) [x86_64-linux])
• Backport deleted (2.1: UNKNOWN, 2.2: UNKNOWN, 2.3: UNKNOWN)