Merge pull request mruby#6560 from mruby/set-gc-marking-v2

Author: matz

include/mruby/internal.h

@@ -133,6 +133,11 @@ mrb_value mrb_as_rational(mrb_state *mrb, mrb_value x);
 void mrb_rational_copy(mrb_state *mrb, mrb_value x, mrb_value y);
 int mrb_rational_mark(mrb_state *mrb, struct RBasic *rat);
 #endif
+#ifdef MRB_USE_SET
+size_t mrb_gc_mark_set(mrb_state *mrb, struct RBasic *set);
+void mrb_gc_free_set(mrb_state *mrb, struct RBasic *set);
+size_t mrb_set_memsize(mrb_value);
+#endif
 
 #ifdef MRUBY_PROC_H
 struct RProc *mrb_closure_new(mrb_state*, const mrb_irep*);

include/mruby/value.h

@@ -163,7 +163,8 @@ static const unsigned int IEEE754_INFINITY_BITS_SINGLE = 0x7F800000;
   f(MRB_TT_COMPLEX,     struct RComplex,    "Complex") \
   f(MRB_TT_RATIONAL,    struct RRational,   "Rational") \
   f(MRB_TT_BIGINT,      struct RBigint,     "Integer") \
-  f(MRB_TT_BACKTRACE,   struct RBacktrace,  "backtrace")
+  f(MRB_TT_BACKTRACE,   struct RBacktrace,  "backtrace") \
+  f(MRB_TT_SET,         struct RSet,        "Set")
 
 enum mrb_vtype {
 #define MRB_VTYPE_DEFINE(tt, type, name) tt,

mrbgems/mruby-bin-mrbc/tools/mrbc/stub.c

@@ -81,3 +81,16 @@ int mrb_rational_mark(mrb_state *mrb, struct RBasic *x)
   return 2;
 }
 #endif
+
+#ifdef MRB_USE_SET
+size_t mrb_gc_mark_set(mrb_state *mrb, struct RBasic *obj)
+{
+  /* stub for mrbc */
+  return 0;
+}
+
+void mrb_gc_free_set(mrb_state *mrb, struct RBasic *obj)
+{
+  /* stub for mrbc */
+}
+#endif

mrbgems/mruby-os-memsize/src/memsize.c

@@ -156,6 +156,11 @@ os_memsize_of_object(mrb_state* mrb, mrb_value obj)
       size += mrb_objspace_page_slot_size();
 #endif
       break;
+#if defined(MRB_USE_SET)
+    case MRB_TT_SET:
+      size += mrb_set_memsize(obj);
+      break;
+#endif
     case MRB_TT_BIGINT:
 #if defined(MRB_USE_BIGINT)
       size += mrb_bint_memsize(obj);

mrbgems/mruby-set/mrbgem.rake

@@ -1,6 +1,7 @@
 MRuby::Gem::Specification.new('mruby-set') do |spec|
   spec.license = 'MIT'
   spec.authors = 'yui-knk'
+  spec.build.defines << "MRB_USE_SET"
 
   spec.add_dependency "mruby-hash-ext", :core => "mruby-hash-ext"
   spec.add_dependency "mruby-enumerator", :core => "mruby-enumerator"

mrbgems/mruby-set/src/set.c

@@ -19,6 +19,11 @@
 KHASH_DECLARE(set, mrb_value, char, FALSE)
 KHASH_DEFINE(set, mrb_value, char, FALSE, mrb_obj_hash_code, mrb_eql)
 
+struct RSet {
+  MRB_OBJECT_HEADER;
+  khash_t(set) *kh;
+};
+
 static void
 set_copy_elements(mrb_state *mrb, khash_t(set) *target_kh, khash_t(set) *source_kh)
 {
@@ -31,31 +36,60 @@ set_copy_elements(mrb_state *mrb, khash_t(set) *target_kh, khash_t(set) *source_
   }
 }
 
-#define SET_KHASH_IV MRB_SYM(khash)
-
+#define mrb_set_ptr(o) ((struct RSet*)mrb_obj_ptr(o))
 static void
-set_free(mrb_state *mrb, void *ptr)
+set_set_khash(mrb_state *mrb, mrb_value self, khash_t(set) *kh)
 {
-  khash_t(set) *kh = (khash_t(set)*)ptr;
-  if (kh) {
-    kh_destroy(set, mrb, kh);
-  }
+  mrb_check_type(mrb, self, MRB_TT_SET);
+  struct RSet *set = mrb_set_ptr(self);
+  set->kh = kh;
 }
 
-static const struct mrb_data_type set_data_type = {
-  "Set", set_free
-};
+static khash_t(set) *
+set_get_khash(mrb_state *mrb, mrb_value self)
+{
+  mrb_check_type(mrb, self, MRB_TT_SET);
+  return mrb_set_ptr(self)->kh;
+}
 
-static void
-set_set_khash(mrb_state *mrb, mrb_value self, khash_t(set) *kh)
+/* Mark function for Set instances */
+size_t
+mrb_gc_mark_set(mrb_state *mrb, struct RBasic *obj)
 {
-  mrb_data_init(self, kh, &set_data_type);
+  struct RSet *s = (struct RSet*)obj;
+  khash_t(set) *kh = s->kh;
+  if (!kh) return 0;
+
+  KHASH_FOREACH(mrb, kh, k) {
+    if (kh_exist(kh, k)) {
+      mrb_gc_mark_value(mrb, kh_key(kh, k));
+    }
+  }
+  return kh_size(kh);
 }
 
-static khash_t(set) *
-set_get_khash(mrb_state *mrb, mrb_value self)
+void
+mrb_gc_free_set(mrb_state *mrb, struct RBasic *obj)
+{
+  struct RSet *s = (struct RSet*)obj;
+  if (s->kh) {
+    khash_t(set) *kh = s->kh;
+    if (kh) {
+      kh_destroy(set, mrb, kh);
+    }
+  }
+}
+
+size_t
+mrb_set_memsize(mrb_value set)
 {
-  return (khash_t(set)*)mrb_data_get_ptr(mrb, self, &set_data_type);
+
+  size_t size = mrb_objspace_page_slot_size();
+  struct RSet *s = mrb_set_ptr(set);
+  if (s->kh) {
+    size += kh_size(s->kh) * sizeof(mrb_value);
+  }
+  return size;
 }
 
 /* Helper function to check if a value is a Set and return a boolean result */
@@ -93,7 +127,10 @@ set_init_copy(mrb_state *mrb, mrb_value self)
   mrb_value orig = mrb_get_arg1(mrb);
   khash_t(set) *kh;
 
-  if (mrb_type(orig) != MRB_TT_CDATA || (DATA_TYPE(self) && DATA_TYPE(self) != DATA_TYPE(orig))) {
+  if (mrb_type(orig) != MRB_TT_SET) {
+    mrb_raise(mrb, E_TYPE_ERROR, "initialize_copy should take a Set object");
+  }
+  if (mrb_obj_class(mrb, self) != mrb_obj_class(mrb, orig)) {
     mrb_raise(mrb, E_TYPE_ERROR, "initialize_copy should take same class object");
   }
 
@@ -1356,7 +1393,7 @@ mrb_mruby_set_gem_init(mrb_state *mrb)
   struct RClass *set;
 
   set = mrb_define_class(mrb, "Set", mrb->object_class);
-  MRB_SET_INSTANCE_TT(set, MRB_TT_CDATA); /* Set instances will hold a C pointer (khash) */
+  MRB_SET_INSTANCE_TT(set, MRB_TT_SET);
 
   mrb_include_module(mrb, set, mrb_module_get(mrb, "Enumerable"));
 

src/gc.c

@@ -729,6 +729,11 @@ gc_mark_children(mrb_state *mrb, mrb_gc *gc, struct RBasic *obj)
     children += mrb_rational_mark(mrb, obj);
     break;
 #endif
+#ifdef MRB_USE_SET
+  case MRB_TT_SET:
+    children += mrb_gc_mark_set(mrb, obj);
+    break;
+#endif
 
   default:
     break;
@@ -841,6 +846,12 @@ obj_free(mrb_state *mrb, struct RBasic *obj, mrb_bool end)
     mrb_gc_free_range(mrb, ((struct RRange*)obj));
     break;
 
+#ifdef MRB_USE_SET
+  case MRB_TT_SET:
+    mrb_gc_free_set(mrb, obj);
+    break;
+#endif
+
   case MRB_TT_CDATA:
     {
       struct RData *d = (struct RData*)obj;