ruby-changes:37781
From: ko1 <ko1@a...>
Date: Fri, 6 Mar 2015 07:20:39 +0900 (JST)
Subject: [ruby-changes:37781] ko1:r49862 (trunk): * internal.h: remove struct method_table_wrapper.
ko1 2015-03-06 07:20:14 +0900 (Fri, 06 Mar 2015) New Revision: 49862 http://svn.ruby-lang.org/cgi-bin/viewvc.cgi?view=revision&revision=49862 Log: * internal.h: remove struct method_table_wrapper. struct method_table_wrapper was introduced to avoid duplicate marking for method tables. For example, `module M1; def foo; end; end` make one method table (mtbl) contains a method `foo`. M1 (T_MODULE) points mtbl. Classes C1 and C2 includes M1, then two T_ICLASS objects are created and they points mtbl too. In this case, three objects (one T_MODULE and two T_ICLASS objects) points same mtbl. On marking phase, these three objects mark same mtbl. To avoid such duplication, struct method_table_wrapper was introduced. However, created two T_ICLASS objects have same or shorter lifetime than M1 (T_MODULE) object. So that we only need to mark mtbl from M1, not from T_ICLASS objects. This patch tries marking only from M1. Note that one `Module#prepend` call creates two T_ICLASS objects. One for refering to a prepending Module object, same as `Module#include`. We don't nedd to care this T_ICLASS. One for moving original mtbl from a prepending class. We need to mark such mtbl from this T_ICLASS object. To mark the mtbl, we need to use `RCLASS_ORIGIN(klass)` on marking from a prepended class `klass`. * class.c: ditto. * eval.c (rb_using_refinement): ditto. * gc.c: ditto. * include/ruby/ruby.h: define m_tbl directly. The definition of struct RClass should be moved to (srcdir)/internal.h. * method.h: remove decl of rb_free_m_tbl_wrapper(). * object.c: use RCLASS_M_TBL() directly. Modified files: trunk/ChangeLog trunk/class.c trunk/eval.c trunk/gc.c trunk/include/ruby/ruby.h trunk/internal.h trunk/method.h trunk/object.c Index: method.h =================================================================== --- method.h (revision 49861) +++ method.h (revision 49862) @@ -139,6 +139,5 @@ VALUE rb_obj_method_location(VALUE obj, https://github.com/ruby/ruby/blob/trunk/method.h#L139 void rb_mark_method_entry(const rb_method_entry_t *me); void rb_free_method_entry(rb_method_entry_t *me); void rb_sweep_method_entry(void *vm); -void rb_free_m_tbl_wrapper(struct method_table_wrapper *wrapper); #endif /* METHOD_H */ Index: include/ruby/ruby.h =================================================================== --- include/ruby/ruby.h (revision 49861) +++ include/ruby/ruby.h (revision 49862) @@ -816,7 +816,7 @@ struct RClass { https://github.com/ruby/ruby/blob/trunk/include/ruby/ruby.h#L816 struct RBasic basic; VALUE super; rb_classext_t *ptr; - struct method_table_wrapper *m_tbl_wrapper; + struct st_table *m_tbl; }; #define RCLASS_SUPER(c) rb_class_get_superclass(c) #define RMODULE_IV_TBL(m) RCLASS_IV_TBL(m) Index: ChangeLog =================================================================== --- ChangeLog (revision 49861) +++ ChangeLog (revision 49862) @@ -1,3 +1,42 @@ https://github.com/ruby/ruby/blob/trunk/ChangeLog#L1 +Fri Mar 6 07:00:44 2015 Koichi Sasada <ko1@a...> + + * internal.h: remove struct method_table_wrapper. + struct method_table_wrapper was introduced to avoid duplicate marking + for method tables. + + For example, `module M1; def foo; end; end` make one method table + (mtbl) contains a method `foo`. M1 (T_MODULE) points mtbl. + Classes C1 and C2 includes M1, then two T_ICLASS objects are created + and they points mtbl too. In this case, three objects (one T_MODULE + and two T_ICLASS objects) points same mtbl. On marking phase, these + three objects mark same mtbl. To avoid such duplication, struct + method_table_wrapper was introduced. + + However, created two T_ICLASS objects have same or shorter lifetime + than M1 (T_MODULE) object. So that we only need to mark mtbl from M1, + not from T_ICLASS objects. This patch tries marking only from M1. + + Note that one `Module#prepend` call creates two T_ICLASS objects. + One for refering to a prepending Module object, same as + `Module#include`. We don't nedd to care this T_ICLASS. + One for moving original mtbl from a prepending class. We need to + mark such mtbl from this T_ICLASS object. To mark the mtbl, + we need to use `RCLASS_ORIGIN(klass)` on marking from a prepended + class `klass`. + + * class.c: ditto. + + * eval.c (rb_using_refinement): ditto. + + * gc.c: ditto. + + * include/ruby/ruby.h: define m_tbl directly. The definition of + struct RClass should be moved to (srcdir)/internal.h. + + * method.h: remove decl of rb_free_m_tbl_wrapper(). + + * object.c: use RCLASS_M_TBL() directly. + Fri Mar 6 02:50:12 2015 NAKAMURA Usaku <usa@r...> * dir.c (replace_real_basename): need to check the return value of Index: object.c =================================================================== --- object.c (revision 49861) +++ object.c (revision 49862) @@ -651,7 +651,7 @@ static VALUE https://github.com/ruby/ruby/blob/trunk/object.c#L651 class_search_ancestor(VALUE cl, VALUE c) { while (cl) { - if (cl == c || RCLASS_M_TBL_WRAPPER(cl) == RCLASS_M_TBL_WRAPPER(c)) + if (cl == c || RCLASS_M_TBL(cl) == RCLASS_M_TBL(c)) return cl; cl = RCLASS_SUPER(cl); } Index: eval.c =================================================================== --- eval.c (revision 49861) +++ eval.c (revision 49862) @@ -1183,8 +1183,8 @@ rb_using_refinement(NODE *cref, VALUE kl https://github.com/ruby/ruby/blob/trunk/eval.c#L1183 c = iclass = rb_include_class_new(module, superclass); RCLASS_REFINED_CLASS(c) = klass; - RCLASS_M_TBL_WRAPPER(OBJ_WB_UNPROTECT(c)) = - RCLASS_M_TBL_WRAPPER(OBJ_WB_UNPROTECT(module)); + RCLASS_M_TBL(OBJ_WB_UNPROTECT(c)) = + RCLASS_M_TBL(OBJ_WB_UNPROTECT(module)); /* TODO: check unprotecting */ module = RCLASS_SUPER(module); while (module && module != klass) { Index: gc.c =================================================================== --- gc.c (revision 49861) +++ gc.c (revision 49862) @@ -1801,17 +1801,10 @@ free_method_entry_i(st_data_t key, st_da https://github.com/ruby/ruby/blob/trunk/gc.c#L1801 static void rb_free_m_tbl(st_table *tbl) { - st_foreach(tbl, free_method_entry_i, 0); - st_free_table(tbl); -} - -void -rb_free_m_tbl_wrapper(struct method_table_wrapper *wrapper) -{ - if (wrapper->tbl) { - rb_free_m_tbl(wrapper->tbl); + if (tbl) { + st_foreach(tbl, free_method_entry_i, 0); + st_free_table(tbl); } - xfree(wrapper); } static int @@ -1888,9 +1881,8 @@ obj_free(rb_objspace_t *objspace, VALUE https://github.com/ruby/ruby/blob/trunk/gc.c#L1881 break; case T_MODULE: case T_CLASS: - if (RCLASS_M_TBL_WRAPPER(obj)) { - rb_free_m_tbl_wrapper(RCLASS_M_TBL_WRAPPER(obj)); - } + rb_free_m_tbl(RCLASS_M_TBL(obj)); + if (RCLASS_IV_TBL(obj)) { st_free_table(RCLASS_IV_TBL(obj)); } @@ -2863,9 +2855,6 @@ obj_memsize_of(VALUE obj, int use_all_ty https://github.com/ruby/ruby/blob/trunk/gc.c#L2855 break; case T_MODULE: case T_CLASS: - if (RCLASS_M_TBL_WRAPPER(obj)) { - size += sizeof(struct method_table_wrapper); - } if (RCLASS_M_TBL(obj)) { size += st_memsize(RCLASS_M_TBL(obj)); } @@ -3837,19 +3826,13 @@ mark_method_entry_i(st_data_t key, st_da https://github.com/ruby/ruby/blob/trunk/gc.c#L3826 } static void -mark_m_tbl_wrapper(rb_objspace_t *objspace, struct method_table_wrapper *wrapper) +mark_m_tbl(rb_objspace_t *objspace, struct st_table *tbl) { - struct mark_tbl_arg arg; - if (!wrapper || !wrapper->tbl) return; - if (LIKELY(objspace->mark_func_data == 0) && !is_incremental_marking(objspace)) { - /* prevent multiple marking during same GC cycle, - * since m_tbl is shared between several T_ICLASS */ - size_t serial = rb_gc_count(); - if (wrapper->serial == serial) return; - wrapper->serial = serial; + if (tbl) { + struct mark_tbl_arg arg; + arg.objspace = objspace; + st_foreach(tbl, mark_method_entry_i, (st_data_t)&arg); } - arg.objspace = objspace; - st_foreach(wrapper->tbl, mark_method_entry_i, (st_data_t)&arg); } static int @@ -4147,10 +4130,11 @@ gc_mark_children(rb_objspace_t *objspace https://github.com/ruby/ruby/blob/trunk/gc.c#L4130 gc_mark(objspace, any->as.basic.klass); switch (BUILTIN_TYPE(obj)) { - case T_ICLASS: case T_CLASS: case T_MODULE: - mark_m_tbl_wrapper(objspace, RCLASS_M_TBL_WRAPPER(obj)); + if (!RCLASS_EXT(obj)) break; + mark_m_tbl(objspace, RCLASS_M_TBL(RCLASS_ORIGIN(obj))); + case T_ICLASS: if (!RCLASS_EXT(obj)) break; mark_tbl(objspace, RCLASS_IV_TBL(obj)); mark_const_tbl(objspace, RCLASS_CONST_TBL(obj)); Index: class.c =================================================================== --- class.c (revision 49861) +++ class.c (revision 49862) @@ -166,7 +166,7 @@ class_alloc(VALUE flags, VALUE klass) https://github.com/ruby/ruby/blob/trunk/class.c#L166 obj->ptr = ALLOC(rb_classext_t); RCLASS_IV_TBL(obj) = 0; RCLASS_CONST_TBL(obj) = 0; - RCLASS_M_TBL_WRAPPER(obj) = 0; + RCLASS_M_TBL(obj) = 0; RCLASS_SET_SUPER((VALUE)obj, 0); RCLASS_ORIGIN(obj) = (VALUE)obj; RCLASS_IV_INDEX_TBL(obj) = 0; @@ -325,10 +325,7 @@ rb_mod_init_copy(VALUE clone, VALUE orig https://github.com/ruby/ruby/blob/trunk/class.c#L325 rb_free_const_table(RCLASS_CONST_TBL(clone)); RCLASS_CONST_TBL(clone) = 0; } - if (RCLASS_M_TBL_WRAPPER(clone)) { - rb_free_m_tbl_wrapper(RCLASS_M_TBL_WRAPPER(clone)); - RCLASS_M_TBL_WRAPPER(clone) = 0; - } + RCLASS_M_TBL(clone) = 0; if (RCLASS_IV_TBL(orig)) { st_data_t id; @@ -794,8 +791,8 @@ rb_include_class_new(VALUE module, VALUE https://github.com/ruby/ruby/blob/trunk/class.c#L791 RCLASS_IV_TBL(klass) = RCLASS_IV_TBL(module); RCLASS_CONST_TBL(klass) = RCLASS_CONST_TBL(module); - RCLASS_M_TBL_WRAPPER(OBJ_WB_UNPROTECT(klass)) = - RCLASS_M_TBL_WRAPPER(OBJ_WB_UNPROTECT(RCLASS_ORIGIN(module))); + RCLASS_M_TBL(OBJ_WB_UNPROTECT(klass)) = + RCLASS_M_TBL(OBJ_WB_UNPROTECT(RCLASS_ORIGIN(module))); /* TODO: unprotected? */ RCLASS_SET_SUPER(klass, super); if (RB_TYPE_P(module, T_ICLASS)) { @@ -855,7 +852,7 @@ include_modules_at(const VALUE klass, VA https://github.com/ruby/ruby/blob/trunk/class.c#L852 for (p = RCLASS_SUPER(klass); p; p = RCLASS_SUPER(p)) { int type = BUILTIN_TYPE(p); if (type == T_ICLASS) { - if (RCLASS_M_TBL_WRAPPER(p) == RCLASS_M_TBL_WRAPPER(module)) { + if (RCLASS_M_TBL(p) == RCLASS_M_TBL(module)) { if (!superclass_seen) { c = p; /* move insertion point */ } @@ -944,7 +941,7 @@ rb_prepend_module(VALUE klass, VALUE mod https://github.com/ruby/ruby/blob/trunk/class.c#L941 RCLASS_SET_SUPER(origin, RCLASS_SUPER(klass)); RCLASS_SET_SUPER(klass, origin); RCLASS_ORIGIN(klass) = origin; - RCLASS_M_TBL_WRAPPER(origin) = RCLASS_M_TBL_WRAPPER(klass); + RCLASS_M_TBL(origin) = RCLASS_M_TBL(klass); RCLASS_M_TBL_INIT(klass); st_foreach(RCLASS_M_TBL(origin), move_refined_method, (st_data_t) RCLASS_M_TBL(klass)); Index: internal.h =================================================================== --- internal.h (revision 49861) +++ internal.h (revision 49862) @@ -316,11 +316,6 @@ struct rb_classext_struct { https://github.com/ruby/ruby/blob/trunk/internal.h#L316 rb_alloc_func_t allocator; }; -struct method_table_wrapper { - st_table *tbl; - size_t serial; -}; - #ifndef BDIGIT # if SIZEOF_INT*2 <= SIZEOF_LONG_LONG # define BDIGIT unsigned int @@ -480,8 +475,7 @@ void rb_class_remove_from_super_subclass https://github.com/ruby/ruby/blob/trunk/internal.h#L475 #define RCLASS_EXT(c) (RCLASS(c)->ptr) #define RCLASS_IV_TBL(c) (RCLASS_EXT(c)->iv_tbl) #define RCLASS_CONST_TBL(c) (RCLASS_EXT(c)->const_tbl) -#define RCLASS_M_TBL_WRAPPER(c) (RCLASS(c)->m_tbl_wrapper) -#define RCLASS_M_TBL(c) (RCLASS_M_TBL_WRAPPER(c) ? RCLASS_M_TBL_WRAPPER(c)->tbl : 0) +#define RCLASS_M_TBL(c) (RCLASS(c)->m_tbl) #define RCLASS_IV_INDEX_TBL(c) (RCLASS_EXT(c)->iv_index_tbl) #define RCLASS_ORIGIN(c) (RCLASS_EXT(c)->origin) #define RCLASS_REFINED_CLASS(c) (RCLASS_EXT(c)->refined_class) @@ -490,11 +484,7 @@ void rb_class_remove_from_super_subclass https://github.com/ruby/ruby/blob/trunk/internal.h#L484 static inline void RCLASS_M_TBL_INIT(VALUE c) { - struct method_table_wrapper *wrapper; - wrapper = ALLOC(struct method_table_wrapper); - wrapper->tbl = st_init_numtable(); - wrapper->serial = 0; - RCLASS_M_TBL_WRAPPER(c) = wrapper; + RCLASS_M_TBL(c) = st_init_numtable(); } #undef RCLASS_SUPER -- ML: ruby-changes@q... Info: http://www.atdot.net/~ko1/quickml/