From 98d091436d302b50e4d047e10bed0a69845d6af4 Mon Sep 17 00:00:00 2001 From: KOBAYASHI Shuji Date: Wed, 3 Feb 2021 23:16:09 +0900 Subject: [PATCH] Reduce memory usage of instance variable table ## Implementation Summary * Only keys and only values of hash table are contiguous to eliminate structure padding. * Change upper limit of `iv_tbl` size to `UINT16_MAX` (it seems to be acceptable in mruby because the total number of classes/modules immediately after starting Redmine is 20,000 or less). * `iv_tbl*` point hash buckets directly. ## Benchmark Summary Only the results of typical situations on 64-bit Word-boxing are present here. For more detailed information, including consideration, see below report (although most of the body is written in Japanese). * https://shuujii.github.io/mruby-iv-benchmark ### Memory Usage Lower value is better. | iv_tbl Size | Baseline | New | Factor | |------------:|---------------:|---------------:|-----------:| | 4 | 88B | 52B | 0.59091x | | 30 | 536B | 388B | 0.72388x | | 100 | 2072B | 1540B | 0.74324x | | 200 | 4120B | 3076B | 0.74660x | Although not mentioned in the above report, the memory usage of `mrbtest` (full-core gembox) is as follows in the result by Valgrind. * Baseline: 108,086 allocs, 16,313,122 bytes allocated * New: 94,273 allocs, 15,875,214 bytes allocated ### Performance Higher value is better. #### `mrb_obj_iv_set` | iv_tbl Size | Baseline | New | Factor | |------------:|---------------:|---------------:|-----------:| | 4 | 88.63003M i/s | 92.60611M i/s | 1.04486x | | 30 | 32.97066M i/s | 25.25095M i/s | 0.76586x | | 100 | 16.33224M i/s | 22.74998M i/s | 1.39295x | | 200 | 5.64484M i/s | 6.79949M i/s | 1.20455x | #### `mrb_obj_iv_get` | iv_tbl Size | Baseline | New | Factor | |------------:|---------------:|---------------:|-----------:| | 4 | 217.58391M i/s | 237.59912M i/s | 1.09199x | | 30 | 139.56195M i/s | 160.49470M i/s | 1.14999x | | 100 | 143.09716M i/s | 190.95047M i/s | 1.33441x | | 200 | 89.75291M i/s | 134.78717M i/s | 1.50176x | ### Binary Size Lower value is better. | File | Baseline | New | Factor | |:------------|---------------:|---------------:|-----------:| | mruby | 697,520B | 697,520B | 1.00000x | | libmruby.a | 1,046,570B | 1,046,682B | 0.99989x | ## Note The address in `struct RObject::iv` may change after initialization because `iv_tbl*` points directly to hash buckets. Therefore, the address cannot be copied and shared when include/prepend. So, when sharing `iv_tbl`, refer to it via the sharing source class. As a result, the following bug have also been fixed. * [An `iv_tbl` is not shared when a class includes or prepends an empty module](https://gist.github.com/shuujii/0ac23fa24b0c55b2c602b534d81e4a95) --- include/mruby/class.h | 5 +- mrbgems/mruby-metaprog/test/metaprog.rb | 56 ++ src/class.c | 13 +- src/variable.c | 653 +++++++++++++----------- test/t/module.rb | 48 +- 5 files changed, 471 insertions(+), 304 deletions(-) diff --git a/include/mruby/class.h b/include/mruby/class.h index b97bc3991..c7c4cda4d 100644 --- a/include/mruby/class.h +++ b/include/mruby/class.h @@ -16,7 +16,10 @@ MRB_BEGIN_DECL struct RClass { MRB_OBJECT_HEADER; - struct iv_tbl *iv; + union { + struct iv_tbl *iv; + struct RClass *iv_c; + }; struct mt_tbl *mt; struct RClass *super; }; diff --git a/mrbgems/mruby-metaprog/test/metaprog.rb b/mrbgems/mruby-metaprog/test/metaprog.rb index 84f4e00a0..0624450f7 100644 --- a/mrbgems/mruby-metaprog/test/metaprog.rb +++ b/mrbgems/mruby-metaprog/test/metaprog.rb @@ -180,6 +180,18 @@ assert('Module#class_variable_defined?', '15.2.2.4.16') do assert_true Test4ClassVariableDefined.class_variable_defined?(:@@cv) assert_false Test4ClassVariableDefined.class_variable_defined?(:@@noexisting) assert_raise(NameError) { Test4ClassVariableDefined.class_variable_defined?("@@2") } + + # shared empty iv_tbl (include) + m = Module.new + c = Class.new{include m} + m.class_variable_set(:@@cv2, 2) + assert_true c.class_variable_defined?(:@@cv2) + + # shared empty iv_tbl (prepend) + m = Module.new + c = Class.new{prepend m} + m.class_variable_set(:@@cv2, 2) + assert_true c.class_variable_defined?(:@@cv2) end assert('Module#class_variable_get', '15.2.2.4.17') do @@ -192,6 +204,26 @@ assert('Module#class_variable_get', '15.2.2.4.17') do %w[@@a? @@! @a a].each do |n| assert_raise(NameError) { Test4ClassVariableGet.class_variable_get(n) } end + + # shared empty iv_tbl (include) + m = Module.new + class Test4ClassVariableGet end + Test4ClassVariableGet.include m + m.class_variable_set(:@@cv2, 2) + assert_equal 2, Test4ClassVariableGet.class_variable_get(:@@cv2) + class Test4ClassVariableGet + assert_equal 2, @@cv2 + end + + # shared empty iv_tbl (prepend) + m = Module.new + class Test4ClassVariableGet end + Test4ClassVariableGet.prepend m + m.class_variable_set(:@@cv2, 2) + assert_equal 2, Test4ClassVariableGet.class_variable_get(:@@cv2) + class Test4ClassVariableGet + assert_equal 2, @@cv2 + end end assert('Module#class_variable_set', '15.2.2.4.18') do @@ -228,6 +260,18 @@ assert('Module#class_variables', '15.2.2.4.19') do assert_equal [:@@var1], Test4ClassVariables1.class_variables assert_equal [:@@var2, :@@var1], Test4ClassVariables2.class_variables + + # shared empty iv_tbl (include) + m = Module.new + c = Class.new{include m} + m.class_variable_set(:@@var3, 3) + assert_equal [:@@var3], c.class_variables + + # shared empty iv_tbl (prepend) + m = Module.new + c = Class.new{prepend m} + m.class_variable_set(:@@var3, 3) + assert_equal [:@@var3], c.class_variables end assert('Module#constants', '15.2.2.4.24') do @@ -243,6 +287,18 @@ assert('Module#constants', '15.2.2.4.24') do assert_equal [ :C ], TestA.constants assert_equal [ :C, :C2 ], $n + + # shared empty iv_tbl (include) + m = Module.new + TestC = Class.new{include m} + m::C3 = 1 + assert_equal [ :C3 ], TestC.constants + + # shared empty iv_tbl (prepend) + m = Module.new + TestC = Class.new{prepend m} + m::C3 = 1 + assert_equal [ :C3 ], TestC.constants end assert('Module#included_modules', '15.2.2.4.30') do diff --git a/src/class.c b/src/class.c index 09e52d24a..264c36f4b 100644 --- a/src/class.c +++ b/src/class.c @@ -1362,11 +1362,16 @@ static struct RClass* include_class_new(mrb_state *mrb, struct RClass *m, struct RClass *super) { struct RClass *ic = (struct RClass*)mrb_obj_alloc(mrb, MRB_TT_ICLASS, mrb->class_class); + struct RClass *iv_c; if (m->tt == MRB_TT_ICLASS) { + iv_c = m->iv_c; m = m->c; } + else { + iv_c = m; + } MRB_CLASS_ORIGIN(m); - ic->iv = m->iv; + ic->iv_c = iv_c; ic->mt = m->mt; ic->super = super; if (m->tt == MRB_TT_ICLASS) { @@ -1484,13 +1489,15 @@ mrb_prepend_module(mrb_state *mrb, struct RClass *c, struct RClass *m) mrb_check_frozen(mrb, c); if (!(c->flags & MRB_FL_CLASS_IS_PREPENDED)) { - struct RClass *c0; + struct RClass *c0, *iv_c; if (c->tt == MRB_TT_ICLASS) { c0 = c->c; + iv_c = c->iv_c; } else { c0 = c; + iv_c = c; } origin = (struct RClass*)mrb_obj_alloc(mrb, MRB_TT_ICLASS, c0); origin->flags |= MRB_FL_CLASS_IS_ORIGIN | MRB_FL_CLASS_IS_INHERITED; @@ -1498,7 +1505,7 @@ mrb_prepend_module(mrb_state *mrb, struct RClass *c, struct RClass *m) c->super = origin; origin->mt = c->mt; c->mt = NULL; - origin->iv = c->iv; + origin->iv_c = iv_c; mrb_field_write_barrier(mrb, (struct RBasic*)c, (struct RBasic*)origin); c->flags |= MRB_FL_CLASS_IS_PREPENDED; } diff --git a/src/variable.c b/src/variable.c index d5a7d90c7..1210cac15 100644 --- a/src/variable.c +++ b/src/variable.c @@ -12,224 +12,315 @@ #include #include -struct iv_elem { - mrb_sym key; - mrb_value val; -}; - -/* Instance variable table structure */ +/* + * Instance variable table structure + * + * Layout: + * iv_tbl* --. + * \ + * +---------+---------+---+-------+-------+---o--------+--------+ + * Type |mrb_value|mrb_value|...|mrb_sym|mrb_sym|...|uint16_t|uint16_t| + * +---------+---------+---+-------+-------+---+--------+--------+ + * Content | val 1 | val 2 |...| sym 1 | sym 2 |...| size | capa_1 | + * +---------+---------+---+-------+-------+---+--------+--------+ + * + * - Only `val` and only `sym` are contiguous to eliminate structure padding. + * - `val` is placed at the beginning to align it at 8-byte boundary. + * - `capa_1` means "capacity - 1". + */ typedef struct iv_tbl { - size_t size; - size_t alloc; - struct iv_elem *table; + char dummy1; + char dummy2[]; } iv_tbl; -/* Creates the instance variable table. */ -static iv_tbl* -iv_new(mrb_state *mrb) +typedef struct iv_tbl_iter { + mrb_sym *syms; + mrb_value *vals; + uint16_t idx; + uint16_t mask; +} iv_tbl_iter; + +#define IV_INIT_CAPA 1 +#define IV_MAX_CAPA (U32(UINT16_MAX) + 1) +#define IV_MAX_SIZE UINT16_MAX +#define IV_EMPTY_SYM UINT32_MAX +#define IV_DELETED_SYM (IV_EMPTY_SYM - 1) + +#define U16(v) ((uint16_t)(v)) +#define U16P(p) ((uint16_t*)(p)) +#define U32(v) ((uint32_t)(v)) + +#define sym_hash_code(sym) U32((sym) ^ ((sym) << 2) ^ ((sym) >> 2)) +#define iv_size_ptr(t) U16P(t) +#define iv_capa_1_ptr(t) U16P((t) + sizeof(uint16_t)) +#define iv_syms(t) ((mrb_sym*)((t) - sizeof(mrb_sym) * iv_capa(t))) +#define iv_vals(t) ((mrb_value*)((t) - iv_offset_for(iv_capa(t)))) + +#define iv_each_by_hash_code(t, hash_code, it_var, code) do { \ + uint32_t capa__ = iv_capa(t); \ + iv_tbl_iter it_var[1]; \ + iv_it_init_by_hash_code(it_var, t, hash_code); \ + for (; capa__; iv_it_next_by_hash_code(it_var), --capa__) { \ + code; \ + } \ +} while (0) + +#define iv_cycle(t, it_var, code) do { \ + iv_tbl_iter it_var[1]; \ + iv_it_init(it_var, t); \ + for (; TRUE; iv_it_next(it_var)) { \ + code; \ + } \ +} while (0) + +static size_t iv_offset_for(uint32_t capa); +static uint32_t iv_capa(const iv_tbl *t); + +static uint16_t +iv_it_idx_for(const iv_tbl_iter *it, uint32_t v) { - iv_tbl *t; + return v & it->mask; +} - t = (iv_tbl*)mrb_malloc(mrb, sizeof(iv_tbl)); - t->size = 0; - t->alloc = 0; - t->table = NULL; +static void +iv_it_init(iv_tbl_iter *it, iv_tbl *t) +{ + it->syms = iv_syms(t); + it->vals = iv_vals(t); + it->idx = 0; +} +static void +iv_it_init_by_hash_code(iv_tbl_iter *it, iv_tbl *t, uint32_t hash_code) +{ + it->mask = *iv_capa_1_ptr(t); + it->syms = iv_syms(t); + it->vals = iv_vals(t); + it->idx = iv_it_idx_for(it, hash_code); +} + +static void +iv_it_next(iv_tbl_iter *it) +{ + ++it->idx; +} + +static void +iv_it_next_by_hash_code(iv_tbl_iter *it) +{ + it->idx = iv_it_idx_for(it, it->idx + 1); +} + +static mrb_sym +iv_it_sym(const iv_tbl_iter *it) +{ + return it->syms[it->idx]; +} + +static mrb_value +iv_it_val(const iv_tbl_iter *it) +{ + return it->vals[it->idx]; +} + +static mrb_bool +iv_it_empty_p(const iv_tbl_iter *it) +{ + return iv_it_sym(it) == IV_EMPTY_SYM; +} + +static mrb_bool +iv_it_active_p(const iv_tbl_iter *it) +{ + return iv_it_sym(it) < IV_DELETED_SYM; +} + +static void +iv_it_set_val(iv_tbl_iter *it, mrb_value val) +{ + it->vals[it->idx] = val; +} + +static void +iv_it_set(iv_tbl_iter *it, mrb_sym sym, mrb_value val) +{ + it->syms[it->idx] = sym; + it->vals[it->idx] = val; +} + +static void +iv_it_delete(iv_tbl_iter *it) +{ + it->syms[it->idx] = IV_DELETED_SYM; +} + +static size_t +iv_offset_for(uint32_t capa) +{ + return (sizeof(mrb_value) + sizeof(mrb_sym)) * capa; +} + +static size_t +iv_byte_size_for(uint32_t capa) +{ + return iv_offset_for(capa) + sizeof(uint16_t) * 2; +} + +static uint16_t +iv_size(const iv_tbl *t) +{ + return *iv_size_ptr(t); +} + +static void +iv_set_size(iv_tbl *t, uint16_t size) +{ + *iv_size_ptr(t) = size; +} + +static uint32_t +iv_capa(const iv_tbl *t) +{ + return U32(*iv_capa_1_ptr(t)) + 1; +} + +static void +iv_set_capa(iv_tbl *t, uint32_t capa) +{ + *iv_capa_1_ptr(t) = U16(capa - 1); +} + +static iv_tbl* +iv_new(mrb_state *mrb, uint32_t capa) +{ + iv_tbl *t = (iv_tbl*)mrb_malloc(mrb, iv_byte_size_for(capa)) + iv_offset_for(capa); + iv_set_size(t, 0); + iv_set_capa(t, capa); + memset(iv_syms(t), 0xff, sizeof(mrb_sym) * capa); return t; } -static void iv_put(mrb_state *mrb, iv_tbl *t, mrb_sym sym, mrb_value val); - static void -iv_rehash(mrb_state *mrb, iv_tbl *t) +iv_expand(mrb_state *mrb, iv_tbl **tp) { - size_t old_alloc = t->alloc; - size_t new_alloc = old_alloc+1; - struct iv_elem *old_table = t->table; - - khash_power2(new_alloc); - if (old_alloc == new_alloc) return; - - t->alloc = new_alloc; - t->size = 0; - t->table = (struct iv_elem*)mrb_calloc(mrb, sizeof(struct iv_elem), new_alloc); - - for (size_t i = 0; i < old_alloc; i++) { - struct iv_elem *slot = &old_table[i]; - - /* key = 0 means empty; val = undef means deleted */ - if (slot->key != 0 && !mrb_undef_p(slot->val)) { - iv_put(mrb, t, slot->key, slot->val); - } - } - mrb_free(mrb, old_table); -} - -#define slot_empty_p(slot) ((slot)->key == 0 && !mrb_undef_p((slot)->val)) - -/* Set the value for the symbol in the instance variable table. */ -static void -iv_put(mrb_state *mrb, iv_tbl *t, mrb_sym sym, mrb_value val) -{ - size_t hash, pos, start; - struct iv_elem *dslot = NULL; - - if (t == NULL) return; - if (t->alloc == 0) { - iv_rehash(mrb, t); - } - hash = kh_int_hash_func(mrb, sym); - start = pos = hash & (t->alloc-1); - for (;;) { - struct iv_elem *slot = &t->table[pos]; - - if (slot->key == sym) { - slot->val = val; - return; - } - else if (slot_empty_p(slot)) { - t->size++; - slot->key = sym; - slot->val = val; - return; - } - else if (!dslot && mrb_undef_p(slot->val)) { /* deleted */ - dslot = slot; - } - pos = (pos+1) & (t->alloc-1); - if (pos == start) { /* not found */ - if (dslot) { - t->size++; - dslot->key = sym; - dslot->val = val; - return; - } - /* no room */ - iv_rehash(mrb, t); - start = pos = hash & (t->alloc-1); - } - } -} - -/* Get a value for a symbol from the instance variable table. */ -static mrb_bool -iv_get(mrb_state *mrb, iv_tbl *t, mrb_sym sym, mrb_value *vp) -{ - size_t hash, pos, start; - - if (t == NULL) return FALSE; - if (t->alloc == 0) return FALSE; - if (t->size == 0) return FALSE; - - hash = kh_int_hash_func(mrb, sym); - start = pos = hash & (t->alloc-1); - for (;;) { - struct iv_elem *slot = &t->table[pos]; - - if (slot->key == sym) { - if (vp) *vp = slot->val; - return TRUE; - } - else if (slot_empty_p(slot)) { - return FALSE; - } - pos = (pos+1) & (t->alloc-1); - if (pos == start) { /* not found */ - return FALSE; - } - } -} - -/* Deletes the value for the symbol from the instance variable table. */ -static mrb_bool -iv_del(mrb_state *mrb, iv_tbl *t, mrb_sym sym, mrb_value *vp) -{ - size_t hash, pos, start; - - if (t == NULL) return FALSE; - if (t->alloc == 0) return FALSE; - if (t->size == 0) return FALSE; - - hash = kh_int_hash_func(mrb, sym); - start = pos = hash & (t->alloc-1); - for (;;) { - struct iv_elem *slot = &t->table[pos]; - - if (slot->key == sym) { - if (vp) *vp = slot->val; - t->size--; - slot->key = 0; - slot->val = mrb_undef_value(); - return TRUE; - } - else if (slot_empty_p(slot)) { - return FALSE; - } - pos = (pos+1) & (t->alloc-1); - if (pos == start) { /* not found */ - return FALSE; - } - } -} - -/* Iterates over the instance variable table. */ -static void -iv_foreach(mrb_state *mrb, iv_tbl *t, mrb_iv_foreach_func *func, void *p) -{ - size_t i; - - if (t == NULL) return; - if (t->alloc == 0) return; - if (t->size == 0) return; - - for (i=0; ialloc; i++) { - struct iv_elem *slot = &t->table[i]; - - if (slot->key && !mrb_undef_p(slot->val)) { - if ((*func)(mrb, slot->key, slot->val, p) != 0) { - return; - } - } - } - return; -} - -/* Get the size of the instance variable table. */ -static size_t -iv_size(mrb_state *mrb, iv_tbl *t) -{ - if (t == NULL) return 0; - return t->size; -} - -/* Copy the instance variable table. */ -static iv_tbl* -iv_copy(mrb_state *mrb, iv_tbl *t) -{ - iv_tbl *t2; - size_t i; - - if (t == NULL) return NULL; - if (t->alloc == 0) return NULL; - if (t->size == 0) return NULL; - - t2 = iv_new(mrb); - for (i=0; ialloc; i++) { - struct iv_elem *slot = &t->table[i]; - - if (slot->key && !mrb_undef_p(slot->val)) { - iv_put(mrb, t2, slot->key, slot->val); - } - } - return t2; + iv_tbl *t = *tp; + uint32_t capa = iv_capa(t), new_capa = capa * 2; + mrb_assert(capa < IV_MAX_CAPA); + mrb_assert(capa == iv_size(t)); + iv_tbl *new_t = iv_new(mrb, new_capa); + iv_set_size(new_t, iv_size(t)); + iv_set_capa(new_t, new_capa); + iv_cycle(t, it, { + mrb_assert(iv_it_active_p(it)); + mrb_sym sym = iv_it_sym(it); + iv_each_by_hash_code(new_t, sym_hash_code(sym), new_it, { + if (!iv_it_empty_p(new_it)) continue; + iv_it_set(new_it, sym, iv_it_val(it)); + break; + }); + if (!--capa) break; + }); + mrb_free(mrb, iv_vals(t)); + *tp = new_t; } /* Free memory of the instance variable table. */ static void iv_free(mrb_state *mrb, iv_tbl *t) { - mrb_free(mrb, t->table); - mrb_free(mrb, t); + if (t) mrb_free(mrb, iv_vals(t)); +} + +/* Set the value for the symbol in the instance variable table. */ +static void +iv_put(mrb_state *mrb, iv_tbl **tp, mrb_sym sym, mrb_value val) +{ + if (!*tp) { + *tp = iv_new(mrb, IV_INIT_CAPA); + if (IV_INIT_CAPA == 1) { + iv_syms(*tp)[0] = sym; + iv_vals(*tp)[0] = val; + iv_set_size(*tp, 1); + return; + } + } + uint16_t size = iv_size(*tp); + uint32_t hash_code = sym_hash_code(sym); + iv_each_by_hash_code(*tp, hash_code, it, { + if (iv_it_sym(it) == sym) { + iv_it_set_val(it, val); + return; + } + if (!iv_it_active_p(it)) { + if (size == IV_MAX_SIZE) mrb_raise(mrb, E_ARGUMENT_ERROR, "iv_tbl too big"); + iv_it_set(it, sym, val); + iv_set_size(*tp, ++size); + return; + } + }); + iv_expand(mrb, tp); + iv_each_by_hash_code(*tp, hash_code, it, { + if (!iv_it_empty_p(it)) continue; + iv_it_set(it, sym, val); + iv_set_size(*tp, ++size); + return; + }); + mrb_assert("not reached"); +} + +/* Get a value for a symbol from the instance variable table. */ +static mrb_bool +iv_get(const iv_tbl *t, mrb_sym sym, mrb_value *valp) +{ + if (!t) return FALSE; + iv_each_by_hash_code((iv_tbl*)t, sym_hash_code(sym), it, { + if (iv_it_empty_p(it)) return FALSE; + if (iv_it_sym(it) != sym) continue; + if (valp) *valp = iv_it_val(it); + return TRUE; + }); + return FALSE; +} + +/* Deletes the value for the symbol from the instance variable table. */ +static mrb_bool +iv_del(iv_tbl *t, mrb_sym sym, mrb_value *valp) +{ + if (!t) return FALSE; + iv_each_by_hash_code(t, sym_hash_code(sym), it, { + if (iv_it_empty_p(it)) return FALSE; + if (iv_it_sym(it) != sym) continue; + if (valp) *valp = iv_it_val(it); + iv_it_delete(it); + iv_set_size(t, iv_size(t) - 1); + return TRUE; + }); + return FALSE; +} + +/* Iterates over the instance variable table. */ +static void +iv_foreach(mrb_state *mrb, iv_tbl *t, mrb_iv_foreach_func *func, void *data) +{ + uint16_t size; + if (!t || (size = iv_size(t)) == 0) return; + iv_cycle(t, it, { + if (!iv_it_active_p(it)) continue; + if (func(mrb, iv_it_sym(it), iv_it_val(it), data) != 0) return; + if (!--size) return; + }); +} + +/* Copy the instance variable table. */ +static iv_tbl* +iv_copy(mrb_state *mrb, const iv_tbl *t) +{ + if (!t) return NULL; + uint32_t capa = iv_capa(t); + size_t byte_size = iv_byte_size_for(capa); + void *p = mrb_malloc(mrb, byte_size); + memcpy(p, iv_vals(t), byte_size); + return (iv_tbl*)p + iv_offset_for(capa); } static int @@ -254,8 +345,7 @@ mrb_gc_mark_gv(mrb_state *mrb) void mrb_gc_free_gv(mrb_state *mrb) { - if (mrb->globals) - iv_free(mrb, mrb->globals); + iv_free(mrb, mrb->globals); } void @@ -267,15 +357,13 @@ mrb_gc_mark_iv(mrb_state *mrb, struct RObject *obj) size_t mrb_gc_mark_iv_size(mrb_state *mrb, struct RObject *obj) { - return iv_size(mrb, obj->iv); + return obj->iv ? iv_size(obj->iv) : 0; } void mrb_gc_free_iv(mrb_state *mrb, struct RObject *obj) { - if (obj->iv) { - iv_free(mrb, obj->iv); - } + iv_free(mrb, obj->iv); } mrb_value @@ -306,12 +394,18 @@ obj_iv_p(mrb_value obj) } } +static iv_tbl** +class_iv_ptr(struct RClass *c) +{ + return c->tt == MRB_TT_ICLASS ? &c->iv_c->iv : &c->iv; +} + MRB_API mrb_value mrb_obj_iv_get(mrb_state *mrb, struct RObject *obj, mrb_sym sym) { mrb_value v; - if (obj->iv && iv_get(mrb, obj->iv, sym, &v)) + if (iv_get(obj->iv, sym, &v)) return v; return mrb_nil_value(); } @@ -331,10 +425,7 @@ void mrb_obj_iv_set_force(mrb_state *mrb, struct RObject *obj, mrb_sym sym, mrb_value v) { assign_class_name(mrb, obj, sym, v); - if (!obj->iv) { - obj->iv = iv_new(mrb); - } - iv_put(mrb, obj->iv, sym, v); + iv_put(mrb, &obj->iv, sym, v); mrb_field_write_barrier_value(mrb, (struct RBasic*)obj, v); } @@ -399,13 +490,7 @@ mrb_iv_set(mrb_state *mrb, mrb_value obj, mrb_sym sym, mrb_value v) MRB_API mrb_bool mrb_obj_iv_defined(mrb_state *mrb, struct RObject *obj, mrb_sym sym) { - iv_tbl *t; - - t = obj->iv; - if (t) { - return iv_get(mrb, t, sym, NULL); - } - return FALSE; + return iv_get(obj->iv, sym, NULL); } MRB_API mrb_bool @@ -442,14 +527,10 @@ mrb_iv_copy(mrb_state *mrb, mrb_value dest, mrb_value src) struct RObject *d = mrb_obj_ptr(dest); struct RObject *s = mrb_obj_ptr(src); - if (d->iv) { - iv_free(mrb, d->iv); - d->iv = 0; - } - if (s->iv) { - mrb_write_barrier(mrb, (struct RBasic*)d); - d->iv = iv_copy(mrb, s->iv); - } + iv_tbl *t = iv_copy(mrb, s->iv); + iv_free(mrb, d->iv); + if (t) mrb_write_barrier(mrb, (struct RBasic*)d); + d->iv = t; } static int @@ -486,9 +567,8 @@ mrb_value mrb_obj_iv_inspect(mrb_state *mrb, struct RObject *obj) { iv_tbl *t = obj->iv; - size_t len = iv_size(mrb, t); - if (len > 0) { + if (t && iv_size(t) > 0) { const char *cn = mrb_obj_classname(mrb, mrb_obj_value(obj)); mrb_value str = mrb_str_new_capa(mrb, 30); @@ -512,7 +592,7 @@ mrb_iv_remove(mrb_state *mrb, mrb_value obj, mrb_sym sym) mrb_value val; mrb_check_frozen(mrb, mrb_obj_ptr(obj)); - if (iv_del(mrb, t, sym, &val)) { + if (iv_del(t, sym, &val)) { return val; } } @@ -604,10 +684,11 @@ mrb_mod_class_variables(mrb_state *mrb, mrb_value mod) mrb_get_args(mrb, "|b", &inherit); ary = mrb_ary_new(mrb); c = mrb_class_ptr(mod); - while (c) { - iv_foreach(mrb, c->iv, cv_i, &ary); - if (!inherit) break; - c = c->super; + iv_foreach(mrb, c->iv, cv_i, &ary); + if (inherit) { + while ((c = c->super)) { + iv_foreach(mrb, *class_iv_ptr(c), cv_i, &ary); + } } return ary; } @@ -619,26 +700,19 @@ mrb_mod_cv_get(mrb_state *mrb, struct RClass *c, mrb_sym sym) mrb_value v; int given = FALSE; - while (c) { - if (c->iv && iv_get(mrb, c->iv, sym, &v)) { - given = TRUE; - } - c = c->super; - } + do { + if (iv_get(*class_iv_ptr(c), sym, &v)) given = TRUE; + } while ((c = c->super)); if (given) return v; - if (cls && cls->tt == MRB_TT_SCLASS) { + if (cls->tt == MRB_TT_SCLASS) { mrb_value klass; klass = mrb_obj_iv_get(mrb, (struct RObject *)cls, MRB_SYM(__attached__)); c = mrb_class_ptr(klass); if (c->tt == MRB_TT_CLASS || c->tt == MRB_TT_MODULE) { - given = FALSE; - while (c) { - if (c->iv && iv_get(mrb, c->iv, sym, &v)) { - given = TRUE; - } - c = c->super; - } + do { + if (iv_get(*class_iv_ptr(c), sym, &v)) given = TRUE; + } while ((c = c->super)); if (given) return v; } } @@ -658,19 +732,17 @@ mrb_mod_cv_set(mrb_state *mrb, struct RClass *c, mrb_sym sym, mrb_value v) { struct RClass * cls = c; - while (c) { - iv_tbl *t = c->iv; - - if (iv_get(mrb, t, sym, NULL)) { + do { + iv_tbl **iv_ptr = class_iv_ptr(c); + if (iv_get(*iv_ptr, sym, NULL)) { mrb_check_frozen(mrb, c); - iv_put(mrb, t, sym, v); + iv_put(mrb, iv_ptr, sym, v); mrb_field_write_barrier_value(mrb, (struct RBasic*)c, v); return; } - c = c->super; - } + } while ((c = c->super)); - if (cls && cls->tt == MRB_TT_SCLASS) { + if (cls->tt == MRB_TT_SCLASS) { mrb_value klass; klass = mrb_obj_iv_get(mrb, (struct RObject*)cls, MRB_SYM(__attached__)); @@ -690,11 +762,7 @@ mrb_mod_cv_set(mrb_state *mrb, struct RClass *c, mrb_sym sym, mrb_value v) } mrb_check_frozen(mrb, c); - if (!c->iv) { - c->iv = iv_new(mrb); - } - - iv_put(mrb, c->iv, sym, v); + iv_put(mrb, class_iv_ptr(c), sym, v); mrb_field_write_barrier_value(mrb, (struct RBasic*)c, v); } @@ -707,11 +775,9 @@ mrb_cv_set(mrb_state *mrb, mrb_value mod, mrb_sym sym, mrb_value v) mrb_bool mrb_mod_cv_defined(mrb_state *mrb, struct RClass * c, mrb_sym sym) { - while (c) { - iv_tbl *t = c->iv; - if (iv_get(mrb, t, sym, NULL)) return TRUE; - c = c->super; - } + do { + if (iv_get(*class_iv_ptr(c), sym, NULL)) return TRUE; + } while ((c = c->super)); return FALSE; } @@ -774,13 +840,9 @@ const_get(mrb_state *mrb, struct RClass *base, mrb_sym sym) mrb_value name; L_RETRY: - while (c) { - if (c->iv) { - if (iv_get(mrb, c->iv, sym, &v)) - return v; - } - c = c->super; - } + do { + if (iv_get(*class_iv_ptr(c), sym, &v)) return v; + } while ((c = c->super)); if (!retry && base->tt == MRB_TT_MODULE) { c = mrb->object_class; retry = TRUE; @@ -807,14 +869,14 @@ mrb_vm_const_get(mrb_state *mrb, mrb_sym sym) c = MRB_PROC_TARGET_CLASS(mrb->c->ci->proc); if (!c) c = mrb->object_class; - if (iv_get(mrb, c->iv, sym, &v)) { + if (iv_get(c->iv, sym, &v)) { return v; } c2 = c; while (c2 && c2->tt == MRB_TT_SCLASS) { mrb_value klass; - if (!iv_get(mrb, c2->iv, MRB_SYM(__attached__), &klass)) { + if (!iv_get(c2->iv, MRB_SYM(__attached__), &klass)) { c2 = NULL; break; } @@ -825,7 +887,7 @@ mrb_vm_const_get(mrb_state *mrb, mrb_sym sym) proc = mrb->c->ci->proc; while (proc) { c2 = MRB_PROC_TARGET_CLASS(proc); - if (c2 && iv_get(mrb, c2->iv, sym, &v)) { + if (c2 && iv_get(c2->iv, sym, &v)) { return v; } proc = proc->upper; @@ -917,11 +979,11 @@ mrb_mod_constants(mrb_state *mrb, mrb_value mod) mrb_get_args(mrb, "|b", &inherit); ary = mrb_ary_new(mrb); - while (c) { - iv_foreach(mrb, c->iv, const_i, &ary); - if (!inherit) break; - c = c->super; - if (c == mrb->object_class) break; + iv_foreach(mrb, c->iv, const_i, &ary); + if (inherit) { + while ((c = c->super) && c != mrb->object_class) { + iv_foreach(mrb, *class_iv_ptr(c), const_i, &ary); + } } return ary; } @@ -931,7 +993,7 @@ mrb_gv_get(mrb_state *mrb, mrb_sym sym) { mrb_value v; - if (iv_get(mrb, mrb->globals, sym, &v)) + if (iv_get(mrb->globals, sym, &v)) return v; return mrb_nil_value(); } @@ -939,19 +1001,13 @@ mrb_gv_get(mrb_state *mrb, mrb_sym sym) MRB_API void mrb_gv_set(mrb_state *mrb, mrb_sym sym, mrb_value v) { - iv_tbl *t; - - if (!mrb->globals) { - mrb->globals = iv_new(mrb); - } - t = mrb->globals; - iv_put(mrb, t, sym, v); + iv_put(mrb, &mrb->globals, sym, v); } MRB_API void mrb_gv_remove(mrb_state *mrb, mrb_sym sym) { - iv_del(mrb, mrb->globals, sym, NULL); + iv_del(mrb->globals, sym, NULL); } static int @@ -993,12 +1049,11 @@ mrb_const_defined_0(mrb_state *mrb, mrb_value mod, mrb_sym id, mrb_bool exclude, tmp = klass; retry: - while (tmp) { - if (iv_get(mrb, tmp->iv, id, NULL)) { - return TRUE; + if (iv_get(tmp->iv, id, NULL)) return TRUE; + if (recurse || klass == mrb->object_class) { + while ((tmp = tmp->super)) { + if (iv_get(*class_iv_ptr(tmp), id, NULL)) return TRUE; } - if (!recurse && (klass != mrb->object_class)) break; - tmp = tmp->super; } if (!exclude && !mod_retry && (klass->tt == MRB_TT_MODULE)) { mod_retry = TRUE; @@ -1112,8 +1167,8 @@ mrb_class_find_path(mrb_state *mrb, struct RClass *c) str = mrb_sym_name_len(mrb, name, &len); mrb_str_cat(mrb, path, str, len); if (RSTRING_PTR(path)[0] != '#') { - iv_del(mrb, c->iv, MRB_SYM(__outer__), NULL); - iv_put(mrb, c->iv, MRB_SYM(__classname__), path); + iv_del(c->iv, MRB_SYM(__outer__), NULL); + iv_put(mrb, &c->iv, MRB_SYM(__classname__), path); mrb_field_write_barrier_value(mrb, (struct RBasic*)c, path); path = mrb_str_dup(mrb, path); } @@ -1124,8 +1179,8 @@ size_t mrb_obj_iv_tbl_memsize(mrb_value obj) { iv_tbl *t = mrb_obj_ptr(obj)->iv; - if (t == NULL) return 0; - return sizeof(iv_tbl) + t->alloc*sizeof(struct iv_elem); + if (!t) return 0; + return iv_byte_size_for(iv_capa(t)); } #define identchar(c) (ISALNUM(c) || (c) == '_' || !ISASCII(c)) diff --git a/test/t/module.rb b/test/t/module.rb index 07d550804..3bf50879d 100644 --- a/test/t/module.rb +++ b/test/t/module.rb @@ -231,6 +231,18 @@ assert('Module#const_defined?', '15.2.2.4.20') do assert_true Test4ConstDefined.const_defined?(:Const4Test4ConstDefined) assert_false Test4ConstDefined.const_defined?(:NotExisting) assert_wrong_const_name{ Test4ConstDefined.const_defined?(:wrong_name) } + + # shared empty iv_tbl (include) + m = Module.new + c = Class.new{include m} + m::CONST = 1 + assert_true c.const_defined?(:CONST) + + # shared empty iv_tbl (prepend) + m = Module.new + c = Class.new{prepend m} + m::CONST = 1 + assert_true c.const_defined?(:CONST) end assert('Module#const_get', '15.2.2.4.21') do @@ -246,6 +258,18 @@ assert('Module#const_get', '15.2.2.4.21') do assert_uninitialized_const{ Test4ConstGet.const_get(:I_DO_NOT_EXIST) } assert_uninitialized_const{ Test4ConstGet.const_get("I_DO_NOT_EXIST::ME_NEITHER") } assert_wrong_const_name{ Test4ConstGet.const_get(:wrong_name) } + + # shared empty iv_tbl (include) + m = Module.new + c = Class.new{include m} + m::CONST = 1 + assert_equal 1, c.const_get(:CONST) + + # shared empty iv_tbl (prepend) + m = Module.new + c = Class.new{prepend m} + m::CONST = 1 + assert_equal 1, c.const_get(:CONST) end assert('Module#const_set', '15.2.2.4.23') do @@ -779,7 +803,7 @@ end assert('module to return the last value') do m = module M; :m end - assert_equal(m, :m) + assert_equal(:m, m) end assert('module to return nil if body is empty') do @@ -796,3 +820,25 @@ assert('get constant of parent module in singleton class; issue #3568') do assert_equal("value", actual) end + +assert('shared empty iv_tbl (include)') do + m1 = Module.new + m2 = Module.new{include m1} + c = Class.new{include m2} + m1::CONST1 = 1 + assert_equal 1, m2::CONST1 + assert_equal 1, c::CONST1 + m2::CONST2 = 2 + assert_equal 2, c::CONST2 +end + +assert('shared empty iv_tbl (prepend)') do + m1 = Module.new + m2 = Module.new{prepend m1} + c = Class.new{include m2} + m1::CONST1 = 1 + assert_equal 1, m2::CONST1 + assert_equal 1, c::CONST1 + m2::CONST2 = 2 + assert_equal 2, c::CONST2 +end